From 0f2b8d785724673156461629fa425b719f30eef7 Mon Sep 17 00:00:00 2001 From: Rogee Date: Tue, 18 Aug 2026 14:29:10 +0800 Subject: [PATCH] H-289: add Captain Skill management API (#46) * H-289: add Captain Skill management API * H-289: guard Captain Skill updates with CAS * H-289: version all Captain Skill updates --------- Co-authored-by: Rogee --- backend/internal/app/app.go | 3 + backend/internal/app/bootstrap.go | 3 + .../database/captain_skill_migration_test.go | 80 +++++ .../handler/api/v1/captain_skill_handler.go | 193 +++++++++++ .../api/v1/captain_skill_handler_test.go | 147 ++++++++ backend/internal/model/captain_skill.go | 48 +++ .../internal/repository/captain_skill_repo.go | 217 ++++++++++++ backend/internal/router/router.go | 13 + backend/internal/router/router_test.go | 3 + .../internal/service/captain_skill_service.go | 228 +++++++++++++ .../service/captain_skill_service_test.go | 321 ++++++++++++++++++ .../000080_add_captain_skills.down.sql | 3 + .../000080_add_captain_skills.up.sql | 38 +++ backend/tests/helpers/pg_helper.go | 3 + 14 files changed, 1300 insertions(+) create mode 100644 backend/internal/database/captain_skill_migration_test.go create mode 100644 backend/internal/handler/api/v1/captain_skill_handler.go create mode 100644 backend/internal/handler/api/v1/captain_skill_handler_test.go create mode 100644 backend/internal/model/captain_skill.go create mode 100644 backend/internal/repository/captain_skill_repo.go create mode 100644 backend/internal/service/captain_skill_service.go create mode 100644 backend/internal/service/captain_skill_service_test.go create mode 100644 backend/migrations/000080_add_captain_skills.down.sql create mode 100644 backend/migrations/000080_add_captain_skills.up.sql diff --git a/backend/internal/app/app.go b/backend/internal/app/app.go index 607b387f..d34e3eda 100644 --- a/backend/internal/app/app.go +++ b/backend/internal/app/app.go @@ -252,6 +252,9 @@ func autoMigrate(db *gorm.DB) error { &model.BackgroundJob{}, &model.UserSession{}, &model.CaptainMessageReport{}, + &model.CaptainSkill{}, + &model.CaptainSkillReference{}, + &model.CaptainAssistantSkill{}, // S6: WorkingHour — out-of-office / business hours per inbox &model.WorkingHour{}, &model.ChannelShangwutongConfig{}, diff --git a/backend/internal/app/bootstrap.go b/backend/internal/app/bootstrap.go index ed9fd842..1a51bf84 100644 --- a/backend/internal/app/bootstrap.go +++ b/backend/internal/app/bootstrap.go @@ -190,6 +190,7 @@ func Bootstrap(env string) (*App, error) { captainScenarioRepo := repository.NewCaptainScenarioRepo(db) captainInboxRepo := repository.NewCaptainInboxRepo(db) captainCustomToolRepo := repository.NewCaptainCustomToolRepo(db) + captainSkillRepo := repository.NewCaptainSkillRepo(db) captainAssistantResponseRepo := repository.NewCaptainAssistantResponseRepo(db) captainPreferenceRepo := repository.NewCaptainPreferenceRepo(db) captainAutoReplyRuleRepo := repository.NewCaptainAutoReplyRuleRepo(db) @@ -592,6 +593,7 @@ func Bootstrap(env string) (*App, error) { } captainScenarioService := service.NewCaptainScenarioService(captainScenarioRepo, captainAssistantRepo) captainCustomToolService := service.NewCaptainCustomToolService(captainCustomToolRepo, accountRepo) + captainSkillService := service.NewCaptainSkillService(captainSkillRepo) copilotService := service.NewCopilotService(copilotThreadRepo, copilotMessageRepo, copilotSuggestionRepo, llmProvider, captainAssistantRepo) copilotService.SetWorkerPool(workerPool) captainConversationService := service.NewCaptainConversationService(db, llmProvider) @@ -810,6 +812,7 @@ func Bootstrap(env string) (*App, error) { CaptainDocument: v1.NewCaptainDocumentHandler(captainDocumentService), CaptainScenario: v1.NewCaptainScenarioHandler(captainScenarioService), CaptainCustomTool: v1.NewCaptainCustomToolHandler(captainCustomToolService), + CaptainSkill: v1.NewCaptainSkillHandler(captainSkillService), CaptainTask: v1.NewCaptainTaskHandler(captainTaskService), CaptainPreference: v1.NewCaptainPreferenceHandler(captainPreferenceService), CopilotConfig: v1.NewCopilotConfigHandler(copilotConfigService, captainPreferenceService). diff --git a/backend/internal/database/captain_skill_migration_test.go b/backend/internal/database/captain_skill_migration_test.go new file mode 100644 index 00000000..806bf04c --- /dev/null +++ b/backend/internal/database/captain_skill_migration_test.go @@ -0,0 +1,80 @@ +package database + +import ( + "os" + "path/filepath" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/postgres" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/model" +) + +func TestCaptainSkillSchemaSQLiteRoundTrip(t *testing.T) { + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.CaptainAssistant{}, &model.CaptainSkill{}, &model.CaptainSkillReference{}, &model.CaptainAssistantSkill{})) + for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} { + assert.True(t, db.Migrator().HasTable(table), table) + require.NoError(t, db.Migrator().DropTable(table)) + assert.False(t, db.Migrator().HasTable(table), table) + } +} + +func openCaptainMigrationPostgres(t *testing.T) *gorm.DB { + t.Helper() + dsn := os.Getenv("GOCHAT_TEST_DB_URL") + if dsn == "" { + dsn = "host=localhost port=5432 user=postgres password=postgres dbname=gochat_test sslmode=disable" + } + db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(1) + t.Cleanup(func() { _ = sqlDB.Close() }) + schema := "captain_skill_migration_" + time.Now().Format("20060102150405000000000") + require.NoError(t, db.Exec("CREATE SCHEMA "+schema).Error) + t.Cleanup(func() { _ = db.Exec("DROP SCHEMA " + schema + " CASCADE").Error }) + require.NoError(t, db.Exec("SET search_path TO "+schema).Error) + return db +} + +func TestCaptainSkillPostgresMigrationRoundTrip(t *testing.T) { + if os.Getenv("GOCHAT_TEST_DB") == "sqlite" { + t.Skip("PostgreSQL migration test") + } + db := openCaptainMigrationPostgres(t) + require.NoError(t, db.Exec("CREATE TABLE accounts (id SERIAL PRIMARY KEY); CREATE TABLE captain_assistants (id SERIAL PRIMARY KEY, account_id INTEGER NOT NULL);").Error) + + up, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000080_add_captain_skills.up.sql")) + require.NoError(t, err) + require.NoError(t, db.Exec(string(up)).Error) + for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} { + assert.True(t, db.Migrator().HasTable(table), table) + } + require.NoError(t, db.Exec("INSERT INTO accounts(id) VALUES (1); INSERT INTO captain_assistants(id, account_id) VALUES (1, 1);").Error) + require.NoError(t, db.Exec("INSERT INTO captain_skills(id, account_id, name, description, instructions_md, status) VALUES (1, 1, 'policy', 'Policy', 'Use it', 'active')").Error) + require.NoError(t, db.Exec("INSERT INTO captain_skill_references(skill_id, reference_key, content_md, position) VALUES (1, 'standard', 'Terms', 0)").Error) + require.NoError(t, db.Exec("INSERT INTO captain_assistant_skills(account_id, assistant_id, skill_id) VALUES (1, 1, 1)").Error) + assert.Error(t, db.Exec("INSERT INTO captain_skill_references(skill_id, reference_key, content_md, position) VALUES (1, 'standard', 'Duplicate', 1)").Error) + require.NoError(t, db.Exec("DELETE FROM captain_skills WHERE id = 1").Error) + for _, table := range []string{"captain_skill_references", "captain_assistant_skills"} { + var count int64 + require.NoError(t, db.Table(table).Count(&count).Error) + assert.Zero(t, count, table) + } + + down, err := os.ReadFile(filepath.Join("..", "..", "migrations", "000080_add_captain_skills.down.sql")) + require.NoError(t, err) + require.NoError(t, db.Exec(string(down)).Error) + for _, table := range []string{"captain_skills", "captain_skill_references", "captain_assistant_skills"} { + assert.False(t, db.Migrator().HasTable(table), table) + } +} diff --git a/backend/internal/handler/api/v1/captain_skill_handler.go b/backend/internal/handler/api/v1/captain_skill_handler.go new file mode 100644 index 00000000..34b58b84 --- /dev/null +++ b/backend/internal/handler/api/v1/captain_skill_handler.go @@ -0,0 +1,193 @@ +package v1 + +import ( + "errors" + "net/http" + "strconv" + + "github.com/gin-gonic/gin" + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/service" + "github.com/gochat/gochat/pkg/response" + "gorm.io/gorm" +) + +type CaptainSkillHandler struct{ svc *service.CaptainSkillService } + +func NewCaptainSkillHandler(svc *service.CaptainSkillService) *CaptainSkillHandler { + return &CaptainSkillHandler{svc: svc} +} + +func (h *CaptainSkillHandler) List(c *gin.Context) { + accountID := parseAccountIDParam(c) + if accountID == 0 { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id") + return + } + var assistantID *uint + if raw := c.Query("assistant_id"); raw != "" { + value, err := strconv.ParseUint(raw, 10, 32) + if err != nil || value == 0 { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid assistant_id") + return + } + id := uint(value) + assistantID = &id + } + items, err := h.svc.List(c.Request.Context(), accountID, assistantID) + if err != nil { + renderCaptainSkillError(c, err) + return + } + payload := make([]gin.H, len(items)) + for i := range items { + payload[i] = captainSkillSummaryPayload(items[i]) + } + c.JSON(http.StatusOK, gin.H{"payload": payload, "meta": gin.H{"total_count": len(payload)}}) +} + +func (h *CaptainSkillHandler) Create(c *gin.Context) { + accountID := parseAccountIDParam(c) + if accountID == 0 { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id") + return + } + var req service.CaptainSkillRequest + if err := bindNestedJSONPayload(c, "skill", &req); err != nil { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error()) + return + } + skill, err := h.svc.Create(c.Request.Context(), accountID, &req) + if err != nil { + renderCaptainSkillError(c, err) + return + } + c.JSON(http.StatusOK, captainSkillDetailPayload(skill)) +} + +func (h *CaptainSkillHandler) Get(c *gin.Context) { + accountID, skillID, ok := captainSkillIDs(c) + if !ok { + return + } + skill, err := h.svc.Get(c.Request.Context(), accountID, skillID) + if err != nil { + renderCaptainSkillError(c, err) + return + } + c.JSON(http.StatusOK, captainSkillDetailPayload(skill)) +} + +func (h *CaptainSkillHandler) Update(c *gin.Context) { + accountID, skillID, ok := captainSkillIDs(c) + if !ok { + return + } + var req service.CaptainSkillRequest + if err := bindNestedJSONPayload(c, "skill", &req); err != nil { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrValidation, err.Error()) + return + } + skill, err := h.svc.Update(c.Request.Context(), accountID, skillID, &req) + if err != nil { + renderCaptainSkillError(c, err) + return + } + c.JSON(http.StatusOK, captainSkillDetailPayload(skill)) +} + +func (h *CaptainSkillHandler) Delete(c *gin.Context) { + accountID, skillID, ok := captainSkillIDs(c) + if !ok { + return + } + if err := h.svc.Delete(c.Request.Context(), accountID, skillID); err != nil { + renderCaptainSkillError(c, err) + return + } + response.NoContent(c) +} + +func (h *CaptainSkillHandler) Bind(c *gin.Context) { + accountID, assistantID, skillID, ok := captainAssistantSkillIDs(c) + if !ok { + return + } + if err := h.svc.Bind(c.Request.Context(), accountID, assistantID, skillID); err != nil { + renderCaptainSkillError(c, err) + return + } + response.NoContent(c) +} + +func (h *CaptainSkillHandler) Unbind(c *gin.Context) { + accountID, assistantID, skillID, ok := captainAssistantSkillIDs(c) + if !ok { + return + } + if err := h.svc.Unbind(c.Request.Context(), accountID, assistantID, skillID); err != nil { + renderCaptainSkillError(c, err) + return + } + response.NoContent(c) +} + +func captainSkillIDs(c *gin.Context) (uint, uint, bool) { + accountID := parseAccountIDParam(c) + skillID, err := parseUintAnyParam(c, "skill_id") + if accountID == 0 || err != nil || skillID == 0 { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid account_id or skill_id") + return 0, 0, false + } + return accountID, skillID, true +} + +func captainAssistantSkillIDs(c *gin.Context) (uint, uint, uint, bool) { + accountID, skillID, ok := captainSkillIDs(c) + if !ok { + return 0, 0, 0, false + } + assistantID, err := parseUintAnyParam(c, "assistant_id") + if err != nil || assistantID == 0 { + response.AbortWithStatusError(c, http.StatusBadRequest, response.ErrBadRequest, "invalid assistant_id") + return 0, 0, 0, false + } + return accountID, assistantID, skillID, true +} + +func renderCaptainSkillError(c *gin.Context, err error) { + switch { + case errors.Is(err, gorm.ErrRecordNotFound): + response.AbortWithStatusError(c, http.StatusNotFound, response.ErrNotFound, "skill or assistant not found") + case errors.Is(err, service.ErrCaptainSkillValidation): + c.JSON(http.StatusUnprocessableEntity, gin.H{"error": err.Error()}) + case errors.Is(err, service.ErrCaptainSkillConflict): + c.JSON(http.StatusConflict, gin.H{"error": err.Error()}) + default: + response.AbortWithStatusError(c, http.StatusInternalServerError, response.ErrInternal, "failed to manage captain skill") + } +} + +func captainSkillSummaryPayload(item service.CaptainSkillListItem) gin.H { + skill := item.Skill + return gin.H{ + "id": skill.ID, "name": skill.Name, "description": skill.Description, "status": skill.Status, + "version": skill.Version, "reference_count": item.ReferenceCount, "bound": item.Bound, + "bound_assistant_count": item.BoundAssistantCount, "updated_at": skill.UpdatedAt.Unix(), + } +} + +func captainSkillDetailPayload(skill *model.CaptainSkill) gin.H { + references := make([]gin.H, len(skill.References)) + for i := range skill.References { + reference := skill.References[i] + references[i] = gin.H{ + "id": reference.ID, "reference_key": reference.ReferenceKey, "content_md": reference.ContentMD, "position": reference.Position, + } + } + return gin.H{ + "id": skill.ID, "name": skill.Name, "description": skill.Description, "instructions_md": skill.InstructionsMD, + "status": skill.Status, "version": skill.Version, "references": references, + "created_at": skill.CreatedAt.Unix(), "updated_at": skill.UpdatedAt.Unix(), + } +} diff --git a/backend/internal/handler/api/v1/captain_skill_handler_test.go b/backend/internal/handler/api/v1/captain_skill_handler_test.go new file mode 100644 index 00000000..c0d481a7 --- /dev/null +++ b/backend/internal/handler/api/v1/captain_skill_handler_test.go @@ -0,0 +1,147 @@ +package v1 + +import ( + "bytes" + "encoding/json" + "net/http" + "net/http/httptest" + "strconv" + "testing" + + "github.com/gin-gonic/gin" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/auth" + "github.com/gochat/gochat/internal/middleware" + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" + "github.com/gochat/gochat/internal/service" +) + +func newCaptainSkillHandlerTestRouter(t *testing.T) (*gin.Engine, *gorm.DB, *model.CaptainAssistant) { + t.Helper() + gin.SetMode(gin.TestMode) + db, err := gorm.Open(sqlite.Open("file:"+t.Name()+"?mode=memory&cache=shared"), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.CaptainAssistant{}, &model.CaptainSkill{}, &model.CaptainSkillReference{}, &model.CaptainAssistantSkill{})) + assistant := &model.CaptainAssistant{AccountID: 1, Name: "Assistant", Status: model.AssistantStatusActive} + require.NoError(t, db.Create(assistant).Error) + + handler := NewCaptainSkillHandler(service.NewCaptainSkillService(repository.NewCaptainSkillRepo(db))) + router := gin.New() + router.Use(func(c *gin.Context) { + role := c.GetHeader("X-Test-Role") + if role == "" { + role = "administrator" + } + c.Set("policy_context", auth.NewPolicyContext(1, 1, role, 0, nil)) + c.Next() + }) + captain := router.Group("/api/v1/accounts/:account_id/captain", middleware.RoleCheckAny("administrator", "super_admin")) + skills := captain.Group("/skills") + skills.GET("", handler.List) + skills.POST("", handler.Create) + skills.GET("/:skill_id", handler.Get) + skills.PUT("/:skill_id", handler.Update) + skills.DELETE("/:skill_id", handler.Delete) + assistants := captain.Group("/assistants/:assistant_id/skills") + assistants.POST("/:skill_id", handler.Bind) + assistants.DELETE("/:skill_id", handler.Unbind) + return router, db, assistant +} + +func captainSkillRequest(t *testing.T, router http.Handler, method, path, role string, body any) *httptest.ResponseRecorder { + t.Helper() + var raw []byte + if body != nil { + raw, _ = json.Marshal(body) + } + req := httptest.NewRequest(method, path, bytes.NewReader(raw)) + req.Header.Set("Content-Type", "application/json") + if role != "" { + req.Header.Set("X-Test-Role", role) + } + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + return w +} + +func captainSkillBody(status string) map[string]any { + return map[string]any{"skill": map[string]any{ + "name": "refund-policy", "description": "Refund rules", "instructions_md": "Use the policy.", "status": status, + "references": []map[string]any{{"reference_key": "standard-policy", "content_md": "Standard terms."}}, + }} +} + +func TestCaptainSkillHandlerManagementContract(t *testing.T) { + router, _, assistant := newCaptainSkillHandlerTestRouter(t) + base := "/api/v1/accounts/1/captain" + + w := captainSkillRequest(t, router, http.MethodPost, base+"/skills", "agent", captainSkillBody("active")) + assert.Equal(t, http.StatusForbidden, w.Code) + + w = captainSkillRequest(t, router, http.MethodPost, base+"/skills", "super_admin", captainSkillBody("active")) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var created map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &created)) + skillID := strconv.Itoa(int(created["id"].(float64))) + assert.Equal(t, "active", created["status"]) + require.Len(t, created["references"], 1) + + w = captainSkillRequest(t, router, http.MethodPost, base+"/assistants/"+strconv.Itoa(int(assistant.ID))+"/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusNoContent, w.Code, w.Body.String()) + w = captainSkillRequest(t, router, http.MethodGet, base+"/skills?assistant_id="+strconv.Itoa(int(assistant.ID)), "administrator", nil) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var listed map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &listed)) + payload := listed["payload"].([]any) + require.Len(t, payload, 1) + assert.Equal(t, true, payload[0].(map[string]any)["bound"]) + assert.NotContains(t, payload[0].(map[string]any), "instructions_md") + w = captainSkillRequest(t, router, http.MethodGet, base+"/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusOK, w.Code) + updatedBody := captainSkillBody("active") + updatedBody["skill"].(map[string]any)["description"] = "Updated refund rules" + updatedBody["skill"].(map[string]any)["expected_version"] = 1 + w = captainSkillRequest(t, router, http.MethodPut, base+"/skills/"+skillID, "administrator", updatedBody) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var updated map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &updated)) + assert.Equal(t, float64(2), updated["version"]) + updatedBody["skill"].(map[string]any)["description"] = "Stale update" + w = captainSkillRequest(t, router, http.MethodPut, base+"/skills/"+skillID, "administrator", updatedBody) + assert.Equal(t, http.StatusConflict, w.Code, w.Body.String()) + w = captainSkillRequest(t, router, http.MethodDelete, base+"/assistants/"+strconv.Itoa(int(assistant.ID))+"/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusNoContent, w.Code) + + w = captainSkillRequest(t, router, http.MethodGet, "/api/v1/accounts/2/captain/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusNotFound, w.Code) + w = captainSkillRequest(t, router, http.MethodPost, base+"/assistants/999/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusNotFound, w.Code) +} + +func TestCaptainSkillHandlerValidationAndConflictStatuses(t *testing.T) { + router, _, assistant := newCaptainSkillHandlerTestRouter(t) + base := "/api/v1/accounts/1/captain" + body := captainSkillBody("active") + body["skill"].(map[string]any)["references"] = []map[string]any{{"reference_key": "../unsafe", "content_md": "x"}} + w := captainSkillRequest(t, router, http.MethodPost, base+"/skills", "administrator", body) + assert.Equal(t, http.StatusUnprocessableEntity, w.Code) + w = captainSkillRequest(t, router, http.MethodGet, base+"/skills?assistant_id=bad", "administrator", nil) + assert.Equal(t, http.StatusBadRequest, w.Code) + + w = captainSkillRequest(t, router, http.MethodPost, base+"/skills", "administrator", captainSkillBody("draft")) + require.Equal(t, http.StatusOK, w.Code, w.Body.String()) + var created map[string]any + require.NoError(t, json.Unmarshal(w.Body.Bytes(), &created)) + skillID := strconv.Itoa(int(created["id"].(float64))) + + w = captainSkillRequest(t, router, http.MethodPost, base+"/assistants/"+strconv.Itoa(int(assistant.ID))+"/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusConflict, w.Code) + w = captainSkillRequest(t, router, http.MethodDelete, base+"/skills/"+skillID, "administrator", nil) + assert.Equal(t, http.StatusNoContent, w.Code) +} diff --git a/backend/internal/model/captain_skill.go b/backend/internal/model/captain_skill.go new file mode 100644 index 00000000..befea77e --- /dev/null +++ b/backend/internal/model/captain_skill.go @@ -0,0 +1,48 @@ +package model + +import "time" + +type CaptainSkillStatus string + +const ( + CaptainSkillStatusDraft CaptainSkillStatus = "draft" + CaptainSkillStatusActive CaptainSkillStatus = "active" + CaptainSkillStatusArchived CaptainSkillStatus = "archived" +) + +type CaptainSkill struct { + ID uint `gorm:"primaryKey;autoIncrement" json:"id"` + AccountID uint `gorm:"not null;uniqueIndex:idx_captain_skills_account_name" json:"account_id"` + Name string `gorm:"size:255;not null;uniqueIndex:idx_captain_skills_account_name" json:"name"` + Description string `gorm:"size:1024;not null" json:"description"` + InstructionsMD string `gorm:"type:text;not null" json:"instructions_md"` + Status CaptainSkillStatus `gorm:"size:20;not null;default:draft;index" json:"status"` + Version uint `gorm:"not null;default:1" json:"version"` + References []CaptainSkillReference `gorm:"foreignKey:SkillID;constraint:OnDelete:CASCADE" json:"references,omitempty"` + CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` + UpdatedAt time.Time `gorm:"autoUpdateTime" json:"updated_at"` +} + +func (CaptainSkill) TableName() string { return "captain_skills" } + +type CaptainSkillReference struct { + ID uint `gorm:"primaryKey;autoIncrement" json:"id"` + SkillID uint `gorm:"not null;uniqueIndex:idx_captain_skill_refs_key;uniqueIndex:idx_captain_skill_refs_position" json:"skill_id"` + ReferenceKey string `gorm:"size:255;not null;uniqueIndex:idx_captain_skill_refs_key" json:"reference_key"` + ContentMD string `gorm:"type:text;not null" json:"content_md"` + Position int `gorm:"not null;uniqueIndex:idx_captain_skill_refs_position" json:"position"` + CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` + UpdatedAt time.Time `gorm:"autoUpdateTime" json:"updated_at"` +} + +func (CaptainSkillReference) TableName() string { return "captain_skill_references" } + +type CaptainAssistantSkill struct { + ID uint `gorm:"primaryKey;autoIncrement" json:"id"` + AccountID uint `gorm:"not null;index" json:"account_id"` + AssistantID uint `gorm:"not null;uniqueIndex:idx_captain_assistant_skill" json:"assistant_id"` + SkillID uint `gorm:"not null;uniqueIndex:idx_captain_assistant_skill;index" json:"skill_id"` + CreatedAt time.Time `gorm:"autoCreateTime" json:"created_at"` +} + +func (CaptainAssistantSkill) TableName() string { return "captain_assistant_skills" } diff --git a/backend/internal/repository/captain_skill_repo.go b/backend/internal/repository/captain_skill_repo.go new file mode 100644 index 00000000..2511a0f2 --- /dev/null +++ b/backend/internal/repository/captain_skill_repo.go @@ -0,0 +1,217 @@ +package repository + +import ( + "context" + "errors" + + "github.com/gochat/gochat/internal/model" + "gorm.io/gorm" + "gorm.io/gorm/clause" +) + +const CaptainAssistantActiveSkillLimit = 50 + +type CaptainSkillSummary struct { + Skill model.CaptainSkill `gorm:"embedded"` + ReferenceCount int64 + Bound bool + BoundAssistantCount int64 +} + +type CaptainSkillRepo struct{ db *gorm.DB } + +func NewCaptainSkillRepo(db *gorm.DB) *CaptainSkillRepo { return &CaptainSkillRepo{db: db} } + +func (r *CaptainSkillRepo) Create(ctx context.Context, skill *model.CaptainSkill) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + references := skill.References + skill.References = nil + if err := tx.Create(skill).Error; err != nil { + return err + } + for i := range references { + references[i].SkillID = skill.ID + } + if len(references) > 0 { + if err := tx.Create(&references).Error; err != nil { + return err + } + } + skill.References = references + return nil + }) +} + +func (r *CaptainSkillRepo) Get(ctx context.Context, accountID, skillID uint) (*model.CaptainSkill, error) { + var skill model.CaptainSkill + err := r.db.WithContext(ctx). + Preload("References", func(db *gorm.DB) *gorm.DB { return db.Order("position ASC") }). + Where("account_id = ? AND id = ?", accountID, skillID).First(&skill).Error + return &skill, err +} + +func (r *CaptainSkillRepo) NameExists(ctx context.Context, accountID uint, name string, exceptID uint) (bool, error) { + var count int64 + q := r.db.WithContext(ctx).Model(&model.CaptainSkill{}).Where("account_id = ? AND name = ?", accountID, name) + if exceptID != 0 { + q = q.Where("id <> ?", exceptID) + } + err := q.Count(&count).Error + return count > 0, err +} + +func (r *CaptainSkillRepo) List(ctx context.Context, accountID uint, assistantID *uint) ([]CaptainSkillSummary, error) { + var result []CaptainSkillSummary + var selectedAssistantID uint + if assistantID != nil { + selectedAssistantID = *assistantID + } + err := r.db.WithContext(ctx).Model(&model.CaptainSkill{}). + Select(`captain_skills.*, + (SELECT COUNT(*) FROM captain_skill_references WHERE skill_id = captain_skills.id) AS reference_count, + EXISTS(SELECT 1 FROM captain_assistant_skills WHERE account_id = ? AND assistant_id = ? AND skill_id = captain_skills.id) AS bound, + (SELECT COUNT(*) FROM captain_assistant_skills WHERE account_id = ? AND skill_id = captain_skills.id) AS bound_assistant_count`, accountID, selectedAssistantID, accountID). + Where("account_id = ?", accountID).Order("updated_at DESC").Scan(&result).Error + return result, err +} + +func (r *CaptainSkillRepo) Update(ctx context.Context, skill *model.CaptainSkill, expectedVersion uint, reconcileReferences, enforceActivationLimit bool) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + result := tx.Model(&model.CaptainSkill{}). + Where("account_id = ? AND id = ? AND version = ?", skill.AccountID, skill.ID, expectedVersion). + Updates(map[string]any{ + "name": skill.Name, "description": skill.Description, "instructions_md": skill.InstructionsMD, + "status": skill.Status, "version": skill.Version, + }) + if result.Error != nil { + return result.Error + } + if result.RowsAffected != 1 { + return ErrCaptainSkillVersionConflict + } + if enforceActivationLimit { + var bindings []model.CaptainAssistantSkill + if err := tx.Where("account_id = ? AND skill_id = ?", skill.AccountID, skill.ID).Order("assistant_id ASC").Find(&bindings).Error; err != nil { + return err + } + for _, binding := range bindings { + var assistant model.CaptainAssistant + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Select("id").Where("account_id = ? AND id = ?", skill.AccountID, binding.AssistantID).First(&assistant).Error; err != nil { + return err + } + var count int64 + err := tx.Model(&model.CaptainAssistantSkill{}). + Joins("JOIN captain_skills ON captain_skills.id = captain_assistant_skills.skill_id AND captain_skills.account_id = captain_assistant_skills.account_id"). + Where("captain_assistant_skills.account_id = ? AND assistant_id = ? AND captain_skills.status = ? AND captain_skills.id <> ?", skill.AccountID, binding.AssistantID, model.CaptainSkillStatusActive, skill.ID). + Count(&count).Error + if err != nil { + return err + } + if count >= CaptainAssistantActiveSkillLimit { + return ErrCaptainAssistantSkillLimit + } + } + } + if !reconcileReferences { + return nil + } + if err := tx.Model(&model.CaptainSkillReference{}).Where("skill_id = ?", skill.ID). + Update("position", gorm.Expr("position + ?", 100)).Error; err != nil { + return err + } + keys := make([]string, len(skill.References)) + for i := range skill.References { + keys[i] = skill.References[i].ReferenceKey + } + if len(skill.References) > 0 { + if err := tx.Clauses(clause.OnConflict{ + Columns: []clause.Column{{Name: "skill_id"}, {Name: "reference_key"}}, + DoUpdates: clause.AssignmentColumns([]string{"content_md", "position", "updated_at"}), + }).Create(&skill.References).Error; err != nil { + return err + } + return tx.Where("skill_id = ? AND reference_key NOT IN ?", skill.ID, keys).Delete(&model.CaptainSkillReference{}).Error + } + return tx.Where("skill_id = ?", skill.ID).Delete(&model.CaptainSkillReference{}).Error + }) +} + +func (r *CaptainSkillRepo) Delete(ctx context.Context, accountID, skillID uint) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + var skill model.CaptainSkill + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Select("id", "status").Where("account_id = ? AND id = ?", accountID, skillID).First(&skill).Error; err != nil { + return err + } + var bindings int64 + if err := tx.Model(&model.CaptainAssistantSkill{}).Where("account_id = ? AND skill_id = ?", accountID, skillID).Count(&bindings).Error; err != nil { + return err + } + if skill.Status != model.CaptainSkillStatusDraft || bindings != 0 { + return ErrCaptainSkillCannotDelete + } + if err := tx.Where("skill_id = ?", skillID).Delete(&model.CaptainSkillReference{}).Error; err != nil { + return err + } + result := tx.Where("account_id = ? AND id = ?", accountID, skillID).Delete(&model.CaptainSkill{}) + if result.Error != nil { + return result.Error + } + if result.RowsAffected != 1 { + return gorm.ErrRecordNotFound + } + return nil + }) +} + +func (r *CaptainSkillRepo) AssistantExists(ctx context.Context, accountID, assistantID uint) error { + var assistant model.CaptainAssistant + return r.db.WithContext(ctx).Select("id").Where("account_id = ? AND id = ?", accountID, assistantID).First(&assistant).Error +} + +func (r *CaptainSkillRepo) Bind(ctx context.Context, accountID, assistantID, skillID uint) error { + return r.db.WithContext(ctx).Transaction(func(tx *gorm.DB) error { + var assistant model.CaptainAssistant + if err := tx.Clauses(clause.Locking{Strength: "UPDATE"}).Select("id").Where("account_id = ? AND id = ?", accountID, assistantID).First(&assistant).Error; err != nil { + return err + } + var skill model.CaptainSkill + if err := tx.Select("id", "status").Where("account_id = ? AND id = ?", accountID, skillID).First(&skill).Error; err != nil { + return err + } + if skill.Status != model.CaptainSkillStatusActive { + return ErrCaptainSkillNotActive + } + var existing int64 + if err := tx.Model(&model.CaptainAssistantSkill{}).Where("account_id = ? AND assistant_id = ? AND skill_id = ?", accountID, assistantID, skillID).Count(&existing).Error; err != nil || existing == 1 { + return err + } + var active int64 + if err := tx.Model(&model.CaptainAssistantSkill{}). + Joins("JOIN captain_skills ON captain_skills.id = captain_assistant_skills.skill_id AND captain_skills.account_id = captain_assistant_skills.account_id"). + Where("captain_assistant_skills.account_id = ? AND assistant_id = ? AND captain_skills.status = ?", accountID, assistantID, model.CaptainSkillStatusActive). + Count(&active).Error; err != nil { + return err + } + if active >= CaptainAssistantActiveSkillLimit { + return ErrCaptainAssistantSkillLimit + } + return tx.Create(&model.CaptainAssistantSkill{AccountID: accountID, AssistantID: assistantID, SkillID: skillID}).Error + }) +} + +func (r *CaptainSkillRepo) Unbind(ctx context.Context, accountID, assistantID, skillID uint) error { + if err := r.AssistantExists(ctx, accountID, assistantID); err != nil { + return err + } + if _, err := r.Get(ctx, accountID, skillID); err != nil { + return err + } + return r.db.WithContext(ctx).Where("account_id = ? AND assistant_id = ? AND skill_id = ?", accountID, assistantID, skillID).Delete(&model.CaptainAssistantSkill{}).Error +} + +var ( + ErrCaptainSkillNotActive = errors.New("skill must be active before binding") + ErrCaptainAssistantSkillLimit = errors.New("assistant active skill limit reached") + ErrCaptainSkillCannotDelete = errors.New("only unbound draft skills can be deleted") + ErrCaptainSkillVersionConflict = errors.New("captain skill version conflict") +) diff --git a/backend/internal/router/router.go b/backend/internal/router/router.go index c8a445ca..3063e1a0 100644 --- a/backend/internal/router/router.go +++ b/backend/internal/router/router.go @@ -56,6 +56,7 @@ type Handlers struct { CaptainDocument *v1.CaptainDocumentHandler CaptainScenario *v1.CaptainScenarioHandler CaptainCustomTool *v1.CaptainCustomToolHandler + CaptainSkill *v1.CaptainSkillHandler CaptainTask *v1.CaptainTaskHandler CaptainPreference *v1.CaptainPreferenceHandler CopilotConfig *v1.CopilotConfigHandler @@ -1363,6 +1364,16 @@ func registerV1Routes(g *gin.RouterGroup, h *Handlers) { captain := accountScoped.Group("/captain") { + // Assistant-level Skill catalog management (administrator only). + skills := captain.Group("/skills", middleware.RoleCheckAny("administrator", "super_admin")) + { + skills.GET("", h.CaptainSkill.List) + skills.POST("", h.CaptainSkill.Create) + skills.GET("/:skill_id", h.CaptainSkill.Get) + skills.PUT("/:skill_id", h.CaptainSkill.Update) + skills.DELETE("/:skill_id", h.CaptainSkill.Delete) + } + // Assistant CRUD assistants := captain.Group("/assistants") { @@ -1384,6 +1395,8 @@ func registerV1Routes(g *gin.RouterGroup, h *Handlers) { assistants.POST("/:assistant_id/inboxes", h.CaptainAssistant.AssociateInbox) assistants.DELETE("/:assistant_id/inboxes/:inbox_id", h.CaptainAssistant.DissociateInbox) assistants.POST("/:assistant_id/playground", h.CaptainAssistant.GenerateResponse) + assistants.POST("/:assistant_id/skills/:skill_id", middleware.RoleCheckAny("administrator", "super_admin"), h.CaptainSkill.Bind) + assistants.DELETE("/:assistant_id/skills/:skill_id", middleware.RoleCheckAny("administrator", "super_admin"), h.CaptainSkill.Unbind) // Documents nested under assistant assistantDocs := assistants.Group("/:assistant_id/documents") diff --git a/backend/internal/router/router_test.go b/backend/internal/router/router_test.go index 6a834f24..21658e21 100644 --- a/backend/internal/router/router_test.go +++ b/backend/internal/router/router_test.go @@ -73,6 +73,9 @@ func TestRegisterRoutesBootsWithChatwootParityConflictGroups(t *testing.T) { "GET /api/v1/accounts/:account_id/captain/assistants/tools", "GET /api/v1/accounts/:account_id/captain/assistants/:assistant_id", "PATCH /api/v1/accounts/:account_id/captain/assistants/:assistant_id", + "GET /api/v1/accounts/:account_id/captain/skills", + "POST /api/v1/accounts/:account_id/captain/assistants/:assistant_id/skills/:skill_id", + "DELETE /api/v1/accounts/:account_id/captain/assistants/:assistant_id/skills/:skill_id", "PATCH /api/v1/accounts/:account_id/captain/assistant_responses/:response_id", "PATCH /api/v1/accounts/:account_id/automation_rules/:automation_id", "PATCH /api/v1/accounts/:account_id/labels/:tag_id", diff --git a/backend/internal/service/captain_skill_service.go b/backend/internal/service/captain_skill_service.go new file mode 100644 index 00000000..3b55b2b1 --- /dev/null +++ b/backend/internal/service/captain_skill_service.go @@ -0,0 +1,228 @@ +package service + +import ( + "context" + "errors" + "fmt" + "regexp" + "strings" + + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" +) + +const ( + CaptainSkillMaxInstructionsBytes = 32 * 1024 + CaptainSkillMaxReferences = 20 + CaptainSkillMaxReferenceBytes = 64 * 1024 +) + +var ( + ErrCaptainSkillValidation = errors.New("captain skill validation failed") + ErrCaptainSkillConflict = errors.New("captain skill state conflict") + referenceKeyPattern = regexp.MustCompile(`^[a-z0-9]+(?:-[a-z0-9]+)*$`) +) + +type CaptainSkillReferenceRequest struct { + ReferenceKey string `json:"reference_key"` + ContentMD string `json:"content_md"` +} + +type CaptainSkillRequest struct { + Name string `json:"name"` + Description string `json:"description"` + InstructionsMD string `json:"instructions_md"` + Status model.CaptainSkillStatus `json:"status"` + References []CaptainSkillReferenceRequest `json:"references"` + ExpectedVersion uint `json:"expected_version"` +} + +type CaptainSkillListItem struct { + Skill model.CaptainSkill + ReferenceCount int64 + Bound bool + BoundAssistantCount int64 +} + +type CaptainSkillService struct{ repo *repository.CaptainSkillRepo } + +func NewCaptainSkillService(repo *repository.CaptainSkillRepo) *CaptainSkillService { + return &CaptainSkillService{repo: repo} +} + +func (s *CaptainSkillService) Create(ctx context.Context, accountID uint, req *CaptainSkillRequest) (*model.CaptainSkill, error) { + if req.Status == "" { + req.Status = model.CaptainSkillStatusDraft + } + if err := validateCaptainSkillRequest(req); err != nil { + return nil, err + } + exists, err := s.repo.NameExists(ctx, accountID, strings.TrimSpace(req.Name), 0) + if err != nil { + return nil, err + } + if exists { + return nil, fmt.Errorf("%w: name is already in use", ErrCaptainSkillValidation) + } + skill := captainSkillFromRequest(accountID, 0, 1, req) + if err := s.repo.Create(ctx, skill); err != nil { + if exists, lookupErr := s.repo.NameExists(ctx, accountID, skill.Name, 0); lookupErr == nil && exists { + return nil, fmt.Errorf("%w: name is already in use", ErrCaptainSkillValidation) + } + return nil, err + } + return skill, nil +} + +func (s *CaptainSkillService) Get(ctx context.Context, accountID, skillID uint) (*model.CaptainSkill, error) { + return s.repo.Get(ctx, accountID, skillID) +} + +func (s *CaptainSkillService) List(ctx context.Context, accountID uint, assistantID *uint) ([]CaptainSkillListItem, error) { + if assistantID != nil { + if err := s.repo.AssistantExists(ctx, accountID, *assistantID); err != nil { + return nil, err + } + } + rows, err := s.repo.List(ctx, accountID, assistantID) + if err != nil { + return nil, err + } + result := make([]CaptainSkillListItem, len(rows)) + for i := range rows { + result[i] = CaptainSkillListItem{rows[i].Skill, rows[i].ReferenceCount, rows[i].Bound, rows[i].BoundAssistantCount} + } + return result, nil +} + +func (s *CaptainSkillService) Update(ctx context.Context, accountID, skillID uint, req *CaptainSkillRequest) (*model.CaptainSkill, error) { + if err := validateCaptainSkillRequest(req); err != nil { + return nil, err + } + if req.ExpectedVersion == 0 { + return nil, fmt.Errorf("%w: expected_version is required", ErrCaptainSkillValidation) + } + current, err := s.repo.Get(ctx, accountID, skillID) + if err != nil { + return nil, err + } + if current.Version != req.ExpectedVersion { + return nil, fmt.Errorf("%w: expected version %d, got %d", ErrCaptainSkillConflict, req.ExpectedVersion, current.Version) + } + exists, err := s.repo.NameExists(ctx, accountID, strings.TrimSpace(req.Name), skillID) + if err != nil { + return nil, err + } + if exists { + return nil, fmt.Errorf("%w: name is already in use", ErrCaptainSkillValidation) + } + next := captainSkillFromRequest(accountID, skillID, current.Version, req) + next.CreatedAt = current.CreatedAt + if captainSkillsEqual(current, next) { + return current, nil + } + next.Version++ + referencesChanged := !captainSkillReferencesEqual(current, next) + enforceActivationLimit := current.Status != model.CaptainSkillStatusActive && req.Status == model.CaptainSkillStatusActive + if err := s.repo.Update(ctx, next, req.ExpectedVersion, referencesChanged, enforceActivationLimit); err != nil { + if errors.Is(err, repository.ErrCaptainSkillVersionConflict) { + return nil, fmt.Errorf("%w: %v", ErrCaptainSkillConflict, err) + } + if errors.Is(err, repository.ErrCaptainAssistantSkillLimit) { + return nil, fmt.Errorf("%w: %v", ErrCaptainSkillConflict, err) + } + if exists, lookupErr := s.repo.NameExists(ctx, accountID, next.Name, skillID); lookupErr == nil && exists { + return nil, fmt.Errorf("%w: name is already in use", ErrCaptainSkillValidation) + } + return nil, err + } + return s.repo.Get(ctx, accountID, skillID) +} + +func (s *CaptainSkillService) Delete(ctx context.Context, accountID, skillID uint) error { + err := s.repo.Delete(ctx, accountID, skillID) + if errors.Is(err, repository.ErrCaptainSkillCannotDelete) { + return fmt.Errorf("%w: %v", ErrCaptainSkillConflict, err) + } + return err +} + +func (s *CaptainSkillService) Bind(ctx context.Context, accountID, assistantID, skillID uint) error { + err := s.repo.Bind(ctx, accountID, assistantID, skillID) + if errors.Is(err, repository.ErrCaptainSkillNotActive) || errors.Is(err, repository.ErrCaptainAssistantSkillLimit) { + return fmt.Errorf("%w: %v", ErrCaptainSkillConflict, err) + } + return err +} + +func (s *CaptainSkillService) Unbind(ctx context.Context, accountID, assistantID, skillID uint) error { + return s.repo.Unbind(ctx, accountID, assistantID, skillID) +} + +func validateCaptainSkillRequest(req *CaptainSkillRequest) error { + if req == nil { + return fmt.Errorf("%w: skill is required", ErrCaptainSkillValidation) + } + name, description := strings.TrimSpace(req.Name), strings.TrimSpace(req.Description) + if name == "" || len(name) > 255 { + return fmt.Errorf("%w: name is required and must be at most 255 bytes", ErrCaptainSkillValidation) + } + if description == "" || len(description) > 1024 { + return fmt.Errorf("%w: description is required and must be at most 1024 bytes", ErrCaptainSkillValidation) + } + if req.InstructionsMD == "" || len(req.InstructionsMD) > CaptainSkillMaxInstructionsBytes { + return fmt.Errorf("%w: instructions_md is required and must be at most %d bytes", ErrCaptainSkillValidation, CaptainSkillMaxInstructionsBytes) + } + if req.Status != model.CaptainSkillStatusDraft && req.Status != model.CaptainSkillStatusActive && req.Status != model.CaptainSkillStatusArchived { + return fmt.Errorf("%w: invalid status", ErrCaptainSkillValidation) + } + if len(req.References) > CaptainSkillMaxReferences { + return fmt.Errorf("%w: at most %d references are allowed", ErrCaptainSkillValidation, CaptainSkillMaxReferences) + } + seen := make(map[string]struct{}, len(req.References)) + for _, reference := range req.References { + if len(reference.ReferenceKey) > 255 || !referenceKeyPattern.MatchString(reference.ReferenceKey) { + return fmt.Errorf("%w: invalid reference_key", ErrCaptainSkillValidation) + } + if _, exists := seen[reference.ReferenceKey]; exists { + return fmt.Errorf("%w: duplicate reference_key", ErrCaptainSkillValidation) + } + seen[reference.ReferenceKey] = struct{}{} + if reference.ContentMD == "" || len(reference.ContentMD) > CaptainSkillMaxReferenceBytes { + return fmt.Errorf("%w: reference content_md is required and must be at most %d bytes", ErrCaptainSkillValidation, CaptainSkillMaxReferenceBytes) + } + } + return nil +} + +func captainSkillFromRequest(accountID, skillID, version uint, req *CaptainSkillRequest) *model.CaptainSkill { + skill := &model.CaptainSkill{ + ID: skillID, AccountID: accountID, Name: strings.TrimSpace(req.Name), Description: strings.TrimSpace(req.Description), + InstructionsMD: req.InstructionsMD, Status: req.Status, Version: version, + References: make([]model.CaptainSkillReference, len(req.References)), + } + for i, reference := range req.References { + skill.References[i] = model.CaptainSkillReference{SkillID: skillID, ReferenceKey: reference.ReferenceKey, ContentMD: reference.ContentMD, Position: i} + } + return skill +} + +func captainSkillsEqual(a, b *model.CaptainSkill) bool { + return a.Status == b.Status && captainSkillContentEqual(a, b) +} + +func captainSkillContentEqual(a, b *model.CaptainSkill) bool { + return a.Name == b.Name && a.Description == b.Description && a.InstructionsMD == b.InstructionsMD && captainSkillReferencesEqual(a, b) +} + +func captainSkillReferencesEqual(a, b *model.CaptainSkill) bool { + if len(a.References) != len(b.References) { + return false + } + for i := range a.References { + if a.References[i].ReferenceKey != b.References[i].ReferenceKey || a.References[i].ContentMD != b.References[i].ContentMD || a.References[i].Position != b.References[i].Position { + return false + } + } + return true +} diff --git a/backend/internal/service/captain_skill_service_test.go b/backend/internal/service/captain_skill_service_test.go new file mode 100644 index 00000000..c1c1dd7a --- /dev/null +++ b/backend/internal/service/captain_skill_service_test.go @@ -0,0 +1,321 @@ +package service + +import ( + "context" + "errors" + "fmt" + "os" + "strings" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "gorm.io/driver/postgres" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" + + "github.com/gochat/gochat/internal/model" + "github.com/gochat/gochat/internal/repository" +) + +func newCaptainSkillTestService(t *testing.T) (*CaptainSkillService, *gorm.DB, *model.CaptainAssistant, *model.CaptainAssistant) { + t.Helper() + db, err := gorm.Open(sqlite.Open("file:"+strings.ReplaceAll(t.Name(), "/", "_")+"?mode=memory&cache=shared"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate( + &model.CaptainAssistant{}, + &model.CaptainSkill{}, + &model.CaptainSkillReference{}, + &model.CaptainAssistantSkill{}, + )) + assistants := []*model.CaptainAssistant{ + {AccountID: 1, Name: "Account one", Status: model.AssistantStatusActive}, + {AccountID: 2, Name: "Account two", Status: model.AssistantStatusActive}, + } + for _, assistant := range assistants { + require.NoError(t, db.Create(assistant).Error) + } + return NewCaptainSkillService(repository.NewCaptainSkillRepo(db)), db, assistants[0], assistants[1] +} + +func validCaptainSkillRequest() *CaptainSkillRequest { + return &CaptainSkillRequest{ + Name: "refund-policy", + Description: "Refund rules", + InstructionsMD: "Use the relevant reference.", + Status: model.CaptainSkillStatusActive, + ExpectedVersion: 1, + References: []CaptainSkillReferenceRequest{ + {ReferenceKey: "regional-exceptions", ContentMD: "Region A differs."}, + {ReferenceKey: "standard-policy", ContentMD: "Standard terms."}, + }, + } +} + +func TestCaptainSkillServiceManagementFlow(t *testing.T) { + svc, db, assistant, otherTenantAssistant := newCaptainSkillTestService(t) + ctx := context.Background() + + skill, err := svc.Create(ctx, assistant.AccountID, validCaptainSkillRequest()) + require.NoError(t, err) + assert.Equal(t, uint(1), skill.Version) + require.Len(t, skill.References, 2) + assert.Equal(t, 0, skill.References[0].Position) + firstReferenceID := skill.References[0].ID + + _, err = svc.Get(ctx, otherTenantAssistant.AccountID, skill.ID) + assert.ErrorIs(t, err, gorm.ErrRecordNotFound) + assert.ErrorIs(t, svc.Bind(ctx, assistant.AccountID, otherTenantAssistant.ID, skill.ID), gorm.ErrRecordNotFound) + assert.ErrorIs(t, svc.Bind(ctx, otherTenantAssistant.AccountID, otherTenantAssistant.ID, skill.ID), gorm.ErrRecordNotFound) + + require.NoError(t, svc.Bind(ctx, assistant.AccountID, assistant.ID, skill.ID)) + require.NoError(t, svc.Bind(ctx, assistant.AccountID, assistant.ID, skill.ID)) + list, err := svc.List(ctx, assistant.AccountID, &assistant.ID) + require.NoError(t, err) + require.Len(t, list, 1) + assert.True(t, list[0].Bound) + assert.Equal(t, int64(1), list[0].BoundAssistantCount) + assert.Equal(t, int64(2), list[0].ReferenceCount) + + request := validCaptainSkillRequest() + request.Description = "Updated rules" + request.References = request.References[:1] + updated, err := svc.Update(ctx, assistant.AccountID, skill.ID, request) + require.NoError(t, err) + assert.Equal(t, uint(2), updated.Version) + require.Len(t, updated.References, 1) + assert.Equal(t, firstReferenceID, updated.References[0].ID, "reference_key reconciliation preserves IDs") + + request.ExpectedVersion = updated.Version + unchanged, err := svc.Update(ctx, assistant.AccountID, skill.ID, request) + require.NoError(t, err) + assert.Equal(t, uint(2), unchanged.Version) + request.Status = model.CaptainSkillStatusArchived + archived, err := svc.Update(ctx, assistant.AccountID, skill.ID, request) + require.NoError(t, err) + assert.Equal(t, uint(3), archived.Version) + list, err = svc.List(ctx, assistant.AccountID, &assistant.ID) + require.NoError(t, err) + assert.True(t, list[0].Bound, "archiving preserves the binding") + request.ExpectedVersion = archived.Version + request.Status = model.CaptainSkillStatusActive + active, err := svc.Update(ctx, assistant.AccountID, skill.ID, request) + require.NoError(t, err) + assert.Equal(t, uint(4), active.Version) + assert.ErrorIs(t, svc.Delete(ctx, assistant.AccountID, skill.ID), ErrCaptainSkillConflict) + + require.NoError(t, svc.Unbind(ctx, assistant.AccountID, assistant.ID, skill.ID)) + require.NoError(t, svc.Unbind(ctx, assistant.AccountID, assistant.ID, skill.ID)) + request.ExpectedVersion = active.Version + request.Status = model.CaptainSkillStatusDraft + draft, err := svc.Update(ctx, assistant.AccountID, skill.ID, request) + require.NoError(t, err) + assert.Equal(t, uint(5), draft.Version) + require.NoError(t, svc.Delete(ctx, assistant.AccountID, skill.ID)) + assert.ErrorIs(t, db.First(&model.CaptainSkill{}, skill.ID).Error, gorm.ErrRecordNotFound) +} + +func TestCaptainSkillServiceConcurrentUpdateConflict(t *testing.T) { + svc, db, assistant, _ := newCaptainSkillTestService(t) + sqlDB, err := db.DB() + require.NoError(t, err) + sqlDB.SetMaxOpenConns(1) + skill, err := svc.Create(context.Background(), assistant.AccountID, validCaptainSkillRequest()) + require.NoError(t, err) + t.Run("content and references", func(t *testing.T) { + assertConcurrentCaptainSkillUpdate(t, svc, skill, false) + }) + t.Run("status only", func(t *testing.T) { + draftRequest := validCaptainSkillRequest() + draftRequest.Name = "status-only" + draftRequest.Status = model.CaptainSkillStatusDraft + draft, err := svc.Create(context.Background(), assistant.AccountID, draftRequest) + require.NoError(t, err) + assertConcurrentCaptainSkillUpdate(t, svc, draft, true) + }) +} + +func TestCaptainSkillServicePostgresSmoke(t *testing.T) { + if os.Getenv("GOCHAT_TEST_DB") == "sqlite" { + t.Skip("PostgreSQL service test") + } + dsn := os.Getenv("GOCHAT_TEST_DB_URL") + if dsn == "" { + dsn = "host=localhost port=5432 user=postgres password=postgres dbname=gochat_test sslmode=disable" + } + db, err := gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + adminDB := db + schema := fmt.Sprintf("captain_skill_service_%d", time.Now().UnixNano()) + require.NoError(t, adminDB.Exec("CREATE SCHEMA "+schema).Error) + t.Cleanup(func() { _ = adminDB.Exec("DROP SCHEMA " + schema + " CASCADE").Error }) + if strings.Contains(dsn, "://") { + separator := "?" + if strings.Contains(dsn, "?") { + separator = "&" + } + dsn += separator + "search_path=" + schema + } else { + dsn += " search_path=" + schema + } + db, err = gorm.Open(postgres.Open(dsn), &gorm.Config{Logger: logger.Default.LogMode(logger.Silent)}) + require.NoError(t, err) + require.NoError(t, db.AutoMigrate(&model.CaptainAssistant{}, &model.CaptainSkill{}, &model.CaptainSkillReference{}, &model.CaptainAssistantSkill{})) + assistant := &model.CaptainAssistant{AccountID: 1, Name: "Postgres", Status: model.AssistantStatusActive} + require.NoError(t, db.Create(assistant).Error) + svc := NewCaptainSkillService(repository.NewCaptainSkillRepo(db)) + skill, err := svc.Create(context.Background(), 1, validCaptainSkillRequest()) + require.NoError(t, err) + update := validCaptainSkillRequest() + update.References[0], update.References[1] = update.References[1], update.References[0] + update.References[0].ContentMD = "Updated and reordered." + skill, err = svc.Update(context.Background(), 1, skill.ID, update) + require.NoError(t, err) + assert.Equal(t, uint(2), skill.Version) + assert.Equal(t, "standard-policy", skill.References[0].ReferenceKey) + assertConcurrentCaptainSkillUpdate(t, svc, skill, false) + require.NoError(t, svc.Bind(context.Background(), 1, assistant.ID, skill.ID)) + items, err := svc.List(context.Background(), 1, &assistant.ID) + require.NoError(t, err) + require.Len(t, items, 1) + assert.True(t, items[0].Bound) + draftRequest := validCaptainSkillRequest() + draftRequest.Name = "status-only" + draftRequest.Status = model.CaptainSkillStatusDraft + draft, err := svc.Create(context.Background(), 1, draftRequest) + require.NoError(t, err) + assertConcurrentCaptainSkillUpdate(t, svc, draft, true) +} + +func assertConcurrentCaptainSkillUpdate(t *testing.T, svc *CaptainSkillService, skill *model.CaptainSkill, statusOnly bool) { + t.Helper() + type updateResult struct { + skill *model.CaptainSkill + err error + } + results := make(chan updateResult, 2) + start := make(chan struct{}) + for i := 0; i < 2; i++ { + req := validCaptainSkillRequest() + req.Name = skill.Name + req.ExpectedVersion = skill.Version + if statusOnly { + req.Status = []model.CaptainSkillStatus{model.CaptainSkillStatusActive, model.CaptainSkillStatusArchived}[i] + } else { + req.Description = fmt.Sprintf("Concurrent update %d", i) + req.References[0].ContentMD = fmt.Sprintf("Concurrent reference %d", i) + } + go func() { + <-start + updated, err := svc.Update(context.Background(), skill.AccountID, skill.ID, req) + results <- updateResult{updated, err} + }() + } + close(start) + + var winner *model.CaptainSkill + conflicts := 0 + for i := 0; i < 2; i++ { + result := <-results + if errors.Is(result.err, ErrCaptainSkillConflict) { + conflicts++ + continue + } + require.NoError(t, result.err) + require.Nil(t, winner, "only one concurrent update may succeed") + winner = result.skill + } + require.Equal(t, 1, conflicts) + require.NotNil(t, winner) + persisted, err := svc.Get(context.Background(), skill.AccountID, skill.ID) + require.NoError(t, err) + assert.Equal(t, skill.Version+1, persisted.Version) + if statusOnly { + assert.Equal(t, winner.Status, persisted.Status) + } else { + assert.Equal(t, winner.Description, persisted.Description) + assert.Equal(t, winner.References[0].ContentMD, persisted.References[0].ContentMD) + } +} + +func TestCaptainSkillServiceValidationAndRollback(t *testing.T) { + svc, db, assistant, _ := newCaptainSkillTestService(t) + ctx := context.Background() + + tests := []struct { + name string + mutate func(*CaptainSkillRequest) + }{ + {"invalid status", func(r *CaptainSkillRequest) { r.Status = "invalid" }}, + {"unsafe reference key", func(r *CaptainSkillRequest) { r.References[0].ReferenceKey = "../secret" }}, + {"duplicate reference key", func(r *CaptainSkillRequest) { r.References[1].ReferenceKey = r.References[0].ReferenceKey }}, + {"instructions too large", func(r *CaptainSkillRequest) { + r.InstructionsMD = strings.Repeat("x", CaptainSkillMaxInstructionsBytes+1) + }}, + {"reference too large", func(r *CaptainSkillRequest) { + r.References[0].ContentMD = strings.Repeat("x", CaptainSkillMaxReferenceBytes+1) + }}, + {"too many references", func(r *CaptainSkillRequest) { + r.References = make([]CaptainSkillReferenceRequest, CaptainSkillMaxReferences+1) + for i := range r.References { + r.References[i] = CaptainSkillReferenceRequest{ReferenceKey: "key-" + string(rune('a'+i)), ContentMD: "x"} + } + }}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + req := validCaptainSkillRequest() + tt.mutate(req) + _, err := svc.Create(ctx, assistant.AccountID, req) + assert.ErrorIs(t, err, ErrCaptainSkillValidation) + }) + } + + draft := validCaptainSkillRequest() + draft.Status = model.CaptainSkillStatusDraft + skill, err := svc.Create(ctx, assistant.AccountID, draft) + require.NoError(t, err) + assert.ErrorIs(t, svc.Bind(ctx, assistant.AccountID, assistant.ID, skill.ID), ErrCaptainSkillConflict) + for i := 0; i < 50; i++ { + limited := &model.CaptainSkill{AccountID: assistant.AccountID, Name: fmt.Sprintf("limit-%d", i), Description: "x", InstructionsMD: "x", Status: model.CaptainSkillStatusActive, Version: 1} + require.NoError(t, db.Create(limited).Error) + require.NoError(t, db.Create(&model.CaptainAssistantSkill{AccountID: assistant.AccountID, AssistantID: assistant.ID, SkillID: limited.ID}).Error) + } + overLimit := validCaptainSkillRequest() + overLimit.Name = "over-limit" + overLimitSkill, err := svc.Create(ctx, assistant.AccountID, overLimit) + require.NoError(t, err) + assert.ErrorIs(t, svc.Bind(ctx, assistant.AccountID, assistant.ID, overLimitSkill.ID), ErrCaptainSkillConflict) + archived := validCaptainSkillRequest() + archived.Name = "archived-bound" + archived.Status = model.CaptainSkillStatusArchived + archivedSkill, err := svc.Create(ctx, assistant.AccountID, archived) + require.NoError(t, err) + require.NoError(t, db.Create(&model.CaptainAssistantSkill{AccountID: assistant.AccountID, AssistantID: assistant.ID, SkillID: archivedSkill.ID}).Error) + archived.Status = model.CaptainSkillStatusActive + _, err = svc.Update(ctx, assistant.AccountID, archivedSkill.ID, archived) + assert.ErrorIs(t, err, ErrCaptainSkillConflict) + + callbackName := "test:captain_skill_reference_failure" + require.NoError(t, db.Callback().Create().Before("gorm:create").Register(callbackName, func(tx *gorm.DB) { + if tx.Statement.Table == "captain_skill_references" { + tx.AddError(errors.New("forced reference failure")) + } + })) + t.Cleanup(func() { _ = db.Callback().Create().Remove(callbackName) }) + + update := validCaptainSkillRequest() + update.Description = "must roll back" + update.References[0].ContentMD = "must also roll back" + _, err = svc.Update(ctx, assistant.AccountID, skill.ID, update) + require.ErrorContains(t, err, "forced reference failure") + persisted, err := svc.Get(ctx, assistant.AccountID, skill.ID) + require.NoError(t, err) + assert.Equal(t, uint(1), persisted.Version) + assert.NotEqual(t, "must roll back", persisted.Description) + require.Len(t, persisted.References, 2) +} diff --git a/backend/migrations/000080_add_captain_skills.down.sql b/backend/migrations/000080_add_captain_skills.down.sql new file mode 100644 index 00000000..ea371f10 --- /dev/null +++ b/backend/migrations/000080_add_captain_skills.down.sql @@ -0,0 +1,3 @@ +DROP TABLE IF EXISTS captain_assistant_skills; +DROP TABLE IF EXISTS captain_skill_references; +DROP TABLE IF EXISTS captain_skills; diff --git a/backend/migrations/000080_add_captain_skills.up.sql b/backend/migrations/000080_add_captain_skills.up.sql new file mode 100644 index 00000000..a28fbd44 --- /dev/null +++ b/backend/migrations/000080_add_captain_skills.up.sql @@ -0,0 +1,38 @@ +CREATE TABLE captain_skills ( + id SERIAL PRIMARY KEY, + account_id INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE, + name VARCHAR(255) NOT NULL, + description VARCHAR(1024) NOT NULL, + instructions_md TEXT NOT NULL, + status VARCHAR(20) NOT NULL DEFAULT 'draft' CHECK (status IN ('draft', 'active', 'archived')), + version INTEGER NOT NULL DEFAULT 1 CHECK (version > 0), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE (account_id, name) +); + +CREATE INDEX idx_captain_skills_account_status ON captain_skills(account_id, status); + +CREATE TABLE captain_skill_references ( + id SERIAL PRIMARY KEY, + skill_id INTEGER NOT NULL REFERENCES captain_skills(id) ON DELETE CASCADE, + reference_key VARCHAR(255) NOT NULL, + content_md TEXT NOT NULL, + position INTEGER NOT NULL CHECK (position >= 0), + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + updated_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE (skill_id, reference_key), + UNIQUE (skill_id, position) +); + +CREATE TABLE captain_assistant_skills ( + id SERIAL PRIMARY KEY, + account_id INTEGER NOT NULL REFERENCES accounts(id) ON DELETE CASCADE, + assistant_id INTEGER NOT NULL REFERENCES captain_assistants(id) ON DELETE CASCADE, + skill_id INTEGER NOT NULL REFERENCES captain_skills(id) ON DELETE CASCADE, + created_at TIMESTAMPTZ NOT NULL DEFAULT NOW(), + UNIQUE (assistant_id, skill_id) +); + +CREATE INDEX idx_captain_assistant_skills_account ON captain_assistant_skills(account_id); +CREATE INDEX idx_captain_assistant_skills_skill ON captain_assistant_skills(skill_id); diff --git a/backend/tests/helpers/pg_helper.go b/backend/tests/helpers/pg_helper.go index 361b68e1..d68d6ea5 100644 --- a/backend/tests/helpers/pg_helper.go +++ b/backend/tests/helpers/pg_helper.go @@ -189,6 +189,9 @@ func allModels() []interface{} { &model.CaptainDocument{}, &model.CaptainInbox{}, &model.CaptainScenario{}, + &model.CaptainSkill{}, + &model.CaptainSkillReference{}, + &model.CaptainAssistantSkill{}, // Copilot models &model.CopilotMessage{}, &model.CopilotThread{},