diff --git a/server/cmd/migrate/main.go b/server/cmd/migrate/main.go index aea80d6a..81cba0ea 100644 --- a/server/cmd/migrate/main.go +++ b/server/cmd/migrate/main.go @@ -117,21 +117,72 @@ func main() { } } + if err := prepareSchema(db); err != nil { + log.Fatalf("Failed to prepare schema: %v", err) + } + + if err := backfillCapabilityContentVersioning(db); err != nil { + log.Fatalf("Failed to backfill capability content versioning: %v", err) + } + if err := backfillUserExternalIdentities(db, false); err != nil { + log.Fatalf("Failed to backfill user external identities: %v", err) + } + if err := backfillUserAuthIdentities(db, false); err != nil { + log.Fatalf("Failed to backfill user auth identities: %v", err) + } + if err := backfillOrganizations(db); err != nil { + log.Fatalf("Failed to backfill organizations: %v", err) + } + + log.Println("All migrations completed successfully") +} + +// prepareSchema runs the full schema-preparation sequence in the exact order +// the default migrate path uses: pre-migrations (bootstrap tables, column +// reconciliation, slug dedup) → AutoMigrate → goose migrations → ensure user +// identity columns/table. It is the single source of truth for "get the +// schema current" so the default migrate path and any subcommand that needs +// the schema in place (e.g. ingest-upstream) can never drift in ordering. +// +// Ordering matters: runPreMigrations must precede autoMigrateAll because +// AutoMigrate touches CapabilityItem.Embedding (a pgvector `vector(1024)` +// column) and the pre-migration bootstrap establishes the tables/columns +// AutoMigrate reconciles against. Every step is idempotent, so re-running on +// an already-migrated DB is a no-op. Data backfills (content versioning, +// external identities, organizations) are intentionally NOT part of schema +// prep — they live only in the default migrate path. +func prepareSchema(db *gorm.DB) error { if err := runPreMigrations(db); err != nil { - log.Fatalf("Failed to run pre-migrations: %v", err) + return fmt.Errorf("run pre-migrations: %w", err) + } + if err := autoMigrateAll(db); err != nil { + return fmt.Errorf("auto-migrate database: %w", err) + } + if err := runGooseMigrations(db); err != nil { + return fmt.Errorf("run goose migrations: %w", err) + } + if err := ensureUserIdentityColumns(db); err != nil { + return fmt.Errorf("ensure user identity columns: %w", err) } + if err := ensureUserAuthIdentitiesTable(db); err != nil { + return fmt.Errorf("ensure user auth identities table: %w", err) + } + return nil +} - err = db.AutoMigrate( +// autoMigrateAll runs GORM AutoMigrate over the full model set. It is the +// single source of truth for the model list; callers go through prepareSchema +// rather than invoking this directly so the pre-migration ordering is always +// honored. AutoMigrate is idempotent additive DDL, so re-running it on an +// already-migrated DB is a no-op. +func autoMigrateAll(db *gorm.DB) error { + return db.AutoMigrate( &team.TeamSession{}, &team.TeamSessionMember{}, &team.TeamTask{}, &team.TeamApprovalRequest{}, &team.TeamRepoAffinity{}, &models.UserSystemRole{}, - &models.Repository{}, - &models.RepoMember{}, - &models.RepoInvitation{}, - &models.UserSystemRole{}, &models.Repository{}, &models.RepoMember{}, &models.RepoInvitation{}, @@ -165,34 +216,6 @@ func main() { &models.ItemDistributionReceipt{}, &models.Organization{}, ) - if err != nil { - log.Fatalf("Failed to auto-migrate database: %v", err) - } - - if err := runGooseMigrations(db); err != nil { - log.Fatalf("Failed to run goose migrations: %v", err) - } - if err := ensureUserIdentityColumns(db); err != nil { - log.Fatalf("Failed to ensure user identity columns: %v", err) - } - if err := ensureUserAuthIdentitiesTable(db); err != nil { - log.Fatalf("Failed to ensure user auth identities table: %v", err) - } - - if err := backfillCapabilityContentVersioning(db); err != nil { - log.Fatalf("Failed to backfill capability content versioning: %v", err) - } - if err := backfillUserExternalIdentities(db, false); err != nil { - log.Fatalf("Failed to backfill user external identities: %v", err) - } - if err := backfillUserAuthIdentities(db, false); err != nil { - log.Fatalf("Failed to backfill user auth identities: %v", err) - } - if err := backfillOrganizations(db); err != nil { - log.Fatalf("Failed to backfill organizations: %v", err) - } - - log.Println("All migrations completed successfully") } func printMigrateHelp() { @@ -232,6 +255,35 @@ func ingestUpstreamCatalog(db *gorm.DB, source string, dryRun bool) error { return fmt.Errorf("source is empty") } + // Ensure ONLY the two columns this ingest writes exist, via targeted + // idempotent additive DDL. We deliberately do NOT run the full schema-prep + // sequence (prepareSchema/runPreMigrations/autoMigrateAll/goose) here: + // those mutate row data (repo_id backfill, duplicate-slug renames, goose + // seed/data migrations), which would violate --dry-run safety by changing + // production data before CatalogIngestService ever honors DryRun. The + // supported way to fully migrate a fresh DB is the default `migrate` + // command, which the deploy workflow always runs before `ingest-upstream`. + // Ingest itself only needs to guarantee the new health/evaluation columns + // are present so the item query doesn't fail with "no such column". + // + // Under --dry-run we skip the column-ensure DDL entirely to keep the + // command fully non-mutating (an ALTER TABLE is a persistent schema + // change). If the columns don't exist yet, the dry-run preview against the + // not-yet-migrated DB may report an error or zero counts — that's + // acceptable for a preview; the operator should run the normal `migrate` + // first. + if !dryRun { + healthEvalDDL := []string{ + `ALTER TABLE capability_items ADD COLUMN IF NOT EXISTS health jsonb DEFAULT '{}'`, + `ALTER TABLE capability_items ADD COLUMN IF NOT EXISTS evaluation jsonb DEFAULT '{}'`, + } + for _, stmt := range healthEvalDDL { + if err := db.Exec(stmt).Error; err != nil { + return fmt.Errorf("ensure health/evaluation columns before ingest: %w", err) + } + } + } + svc := &services.CatalogIngestService{ DB: db, Parser: &services.ParserService{}, diff --git a/server/docs/docs.go b/server/docs/docs.go index 4d2c52b2..b4762435 100644 --- a/server/docs/docs.go +++ b/server/docs/docs.go @@ -12962,12 +12962,18 @@ const docTemplate = `{ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14041,12 +14047,18 @@ const docTemplate = `{ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14206,12 +14218,18 @@ const docTemplate = `{ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14700,6 +14718,9 @@ const docTemplate = `{ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, @@ -14709,6 +14730,9 @@ const docTemplate = `{ "favorited": { "type": "boolean" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14819,12 +14843,18 @@ const docTemplate = `{ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, diff --git a/server/docs/swagger.json b/server/docs/swagger.json index e4665d0b..eb5fb39b 100644 --- a/server/docs/swagger.json +++ b/server/docs/swagger.json @@ -12956,12 +12956,18 @@ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14035,12 +14041,18 @@ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14200,12 +14212,18 @@ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14694,6 +14712,9 @@ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, @@ -14703,6 +14724,9 @@ "favorited": { "type": "boolean" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, @@ -14813,12 +14837,18 @@ "embeddingUpdatedAt": { "type": "string" }, + "evaluation": { + "type": "object" + }, "experienceScore": { "type": "number" }, "favoriteCount": { "type": "integer" }, + "health": { + "type": "object" + }, "id": { "type": "string" }, diff --git a/server/docs/swagger.yaml b/server/docs/swagger.yaml index dc6decb8..474c3480 100644 --- a/server/docs/swagger.yaml +++ b/server/docs/swagger.yaml @@ -185,10 +185,14 @@ definitions: type: string embeddingUpdatedAt: type: string + evaluation: + type: object experienceScore: type: number favoriteCount: type: integer + health: + type: object id: type: string installCount: @@ -912,10 +916,14 @@ definitions: type: string embeddingUpdatedAt: type: string + evaluation: + type: object experienceScore: type: number favoriteCount: type: integer + health: + type: object id: type: string installCount: @@ -1022,10 +1030,14 @@ definitions: type: string embeddingUpdatedAt: type: string + evaluation: + type: object experienceScore: type: number favoriteCount: type: integer + health: + type: object id: type: string installCount: @@ -1354,12 +1366,16 @@ definitions: type: string embeddingUpdatedAt: type: string + evaluation: + type: object experienceScore: type: number favoriteCount: type: integer favorited: type: boolean + health: + type: object id: type: string installCount: @@ -1433,10 +1449,14 @@ definitions: type: string embeddingUpdatedAt: type: string + evaluation: + type: object experienceScore: type: number favoriteCount: type: integer + health: + type: object id: type: string installCount: diff --git a/server/internal/handlers/capability_item.go b/server/internal/handlers/capability_item.go index 74561613..8d9d1e15 100644 --- a/server/internal/handlers/capability_item.go +++ b/server/internal/handlers/capability_item.go @@ -490,6 +490,8 @@ type ItemResponse struct { ContentMD5 string `json:"contentMd5"` CurrentRevision int `json:"currentRevision"` Metadata datatypes.JSON `json:"metadata" swaggertype:"object"` + Health datatypes.JSON `json:"health,omitempty" swaggertype:"object"` + Evaluation datatypes.JSON `json:"evaluation,omitempty" swaggertype:"object"` SourcePath string `json:"sourcePath"` SourceSHA string `json:"sourceSha"` SourceType string `json:"sourceType"` @@ -618,6 +620,8 @@ func buildItemResponse(c *gin.Context, db *gorm.DB, item models.CapabilityItem, ContentMD5: item.ContentMD5, CurrentRevision: item.CurrentRevision, Metadata: item.Metadata, + Health: item.Health, + Evaluation: item.Evaluation, SourcePath: item.SourcePath, SourceSHA: item.SourceSHA, SourceType: item.SourceType, diff --git a/server/internal/handlers/capability_item_health_eval_test.go b/server/internal/handlers/capability_item_health_eval_test.go new file mode 100644 index 00000000..0762ab17 --- /dev/null +++ b/server/internal/handlers/capability_item_health_eval_test.go @@ -0,0 +1,91 @@ +package handlers + +import ( + "encoding/json" + "net/http" + "testing" + + "github.com/costrict/costrict-web/server/internal/database" + "github.com/costrict/costrict-web/server/internal/models" + "gorm.io/datatypes" +) + +// TestGetItem_HealthEvaluationPresent verifies the detail endpoint surfaces +// the health and evaluation jsonb blocks (via buildItemResponse) when the +// stored row carries non-empty values. +func TestGetItem_HealthEvaluationPresent(t *testing.T) { + defer setupTestDB(t)() + database.DB.Create(&models.CapabilityRegistry{ + ID: "reg-he1", Name: "he-reg", SourceType: "internal", RepoID: "repo-1", OwnerID: "u1", + }) + health := `{"score":0.82,"freshness_label":"fresh","signals":{"freshness":0.9,"popularity":0.7,"source_trust":0.85}}` + eval := `{"final_score":4.2,"decision":"accept","model_id":"deepseek-v4"}` + database.DB.Create(&models.CapabilityItem{ + ID: "item-he1", RegistryID: "reg-he1", RepoID: "repo-1", Slug: "he-skill", ItemType: "skill", + Name: "HE Skill", Status: "active", CreatedBy: "u1", + Metadata: datatypes.JSON([]byte("{}")), + Health: datatypes.JSON([]byte(health)), + Evaluation: datatypes.JSON([]byte(eval)), + }) + + w := get(newItemRouter(""), "/api/items/item-he1") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var item map[string]interface{} + if err := json.NewDecoder(w.Body).Decode(&item); err != nil { + t.Fatalf("decode body: %v", err) + } + + gotHealth, ok := item["health"].(map[string]interface{}) + if !ok { + t.Fatalf("expected health object in response, got %v", item["health"]) + } + if gotHealth["score"] != float64(0.82) || gotHealth["freshness_label"] != "fresh" { + t.Fatalf("unexpected health payload: %v", gotHealth) + } + + gotEval, ok := item["evaluation"].(map[string]interface{}) + if !ok { + t.Fatalf("expected evaluation object in response, got %v", item["evaluation"]) + } + if gotEval["decision"] != "accept" || gotEval["final_score"] != float64(4.2) { + t.Fatalf("unexpected evaluation payload: %v", gotEval) + } +} + +// TestGetItem_HealthEvaluationOmittedWhenEmpty verifies that an item with no +// health/evaluation data does not surface populated blocks. The DTO uses the +// json `omitempty` tag, so a nil column is dropped from the payload entirely; +// an empty `{}` column (the schema default) round-trips as an empty object. +// Either way the client never sees populated panels for an unscored item. +func TestGetItem_HealthEvaluationOmittedWhenEmpty(t *testing.T) { + defer setupTestDB(t)() + database.DB.Create(&models.CapabilityRegistry{ + ID: "reg-he2", Name: "he-reg2", SourceType: "internal", RepoID: "repo-1", OwnerID: "u1", + }) + // Insert a row whose health/evaluation columns are SQL NULL so that the + // reloaded datatypes.JSON is empty (len 0) and omitempty drops the field. + if err := database.DB.Exec( + `INSERT INTO capability_items (id, registry_id, repo_id, slug, item_type, name, status, created_by, descriptions, metadata, health, evaluation) + VALUES ('item-he2','reg-he2','repo-1','he-empty','skill','HE Empty','active','u1','{}','{}', NULL, NULL)`, + ).Error; err != nil { + t.Fatalf("insert item: %v", err) + } + + w := get(newItemRouter(""), "/api/items/item-he2") + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + var item map[string]interface{} + if err := json.NewDecoder(w.Body).Decode(&item); err != nil { + t.Fatalf("decode body: %v", err) + } + + if v, present := item["health"]; present { + t.Fatalf("expected health to be omitted for empty column, got %v", v) + } + if v, present := item["evaluation"]; present { + t.Fatalf("expected evaluation to be omitted for empty column, got %v", v) + } +} diff --git a/server/internal/handlers/registry_test.go b/server/internal/handlers/registry_test.go index b0c728da..ab89e7a1 100644 --- a/server/internal/handlers/registry_test.go +++ b/server/internal/handlers/registry_test.go @@ -88,6 +88,8 @@ func setupTestDB(t *testing.T) func() { content_md5 TEXT DEFAULT '', current_revision INTEGER NOT NULL DEFAULT 1, metadata TEXT DEFAULT '{}', + health TEXT DEFAULT '{}', + evaluation TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, source_type TEXT NOT NULL DEFAULT 'direct', diff --git a/server/internal/models/models.go b/server/internal/models/models.go index 26a6b581..9fbcf58c 100644 --- a/server/internal/models/models.go +++ b/server/internal/models/models.go @@ -356,6 +356,8 @@ type CapabilityItem struct { ContentMD5 string `gorm:"size:32;default:''" json:"contentMd5"` CurrentRevision int `gorm:"not null;default:1" json:"currentRevision"` Metadata datatypes.JSON `gorm:"type:jsonb;default:'{}'" json:"metadata" swaggertype:"object"` + Health datatypes.JSON `gorm:"type:jsonb;default:'{}'" json:"health" swaggertype:"object"` + Evaluation datatypes.JSON `gorm:"type:jsonb;default:'{}'" json:"evaluation" swaggertype:"object"` SourcePath string `json:"sourcePath"` SourceSHA string `json:"sourceSha"` SourceType string `gorm:"not null;default:'direct'" json:"sourceType"` // direct | archive diff --git a/server/internal/services/catalog_ingest_health_eval_test.go b/server/internal/services/catalog_ingest_health_eval_test.go new file mode 100644 index 00000000..71f80d35 --- /dev/null +++ b/server/internal/services/catalog_ingest_health_eval_test.go @@ -0,0 +1,433 @@ +package services + +import ( + "context" + "encoding/json" + "os" + "path/filepath" + "testing" + + "github.com/costrict/costrict-web/server/internal/models" + "gorm.io/driver/sqlite" + "gorm.io/gorm" + "gorm.io/gorm/logger" +) + +// newIngestTestDB builds an in-memory SQLite DB with exactly the tables the +// CatalogIngestService touches. The schema mirrors the hand-written DDL used +// by scan_service_test.go (so the new health/evaluation columns are present) +// plus the registry / version / tag / category / scan_job tables the ingest +// write paths exercise. +func newIngestTestDB(t *testing.T) *gorm.DB { + t.Helper() + db, err := gorm.Open(sqlite.Open(":memory:"), &gorm.Config{ + Logger: logger.Default.LogMode(logger.Silent), + }) + if err != nil { + t.Fatalf("open test db: %v", err) + } + stmts := []string{ + `CREATE TABLE capability_registries ( + id TEXT PRIMARY KEY, name TEXT NOT NULL, description TEXT, + source_type TEXT NOT NULL DEFAULT 'internal', external_url TEXT, + external_branch TEXT DEFAULT 'main', sync_enabled INTEGER DEFAULT 0, + sync_interval INTEGER DEFAULT 3600, last_synced_at DATETIME, + last_sync_sha TEXT, sync_status TEXT DEFAULT 'idle', + sync_config TEXT DEFAULT '{}', last_sync_log_id TEXT, + repo_id TEXT, owner_id TEXT NOT NULL, + created_at DATETIME, updated_at DATETIME + )`, + `CREATE TABLE capability_items ( + id TEXT PRIMARY KEY, registry_id TEXT NOT NULL, repo_id TEXT NOT NULL, + slug TEXT NOT NULL, item_type TEXT NOT NULL, name TEXT NOT NULL, + description TEXT, descriptions TEXT NOT NULL DEFAULT '{}', category TEXT, + version TEXT DEFAULT '1.0.0', content TEXT, content_md5 TEXT DEFAULT '', + current_revision INTEGER NOT NULL DEFAULT 1, metadata TEXT DEFAULT '{}', + health TEXT DEFAULT '{}', evaluation TEXT DEFAULT '{}', + source_path TEXT, source_sha TEXT, source_type TEXT DEFAULT 'direct', + source TEXT DEFAULT '', preview_count INTEGER DEFAULT 0, + install_count INTEGER DEFAULT 0, favorite_count INTEGER DEFAULT 0, + status TEXT DEFAULT 'active', security_status TEXT DEFAULT 'unscanned', + last_scan_id TEXT, created_by TEXT NOT NULL, updated_by TEXT, + embedding TEXT, experience_score REAL DEFAULT 0, + embedding_updated_at DATETIME, created_at DATETIME, updated_at DATETIME, + UNIQUE(repo_id, item_type, slug) + )`, + `CREATE TABLE capability_versions ( + id TEXT PRIMARY KEY, item_id TEXT NOT NULL, revision INTEGER NOT NULL, + name TEXT, description TEXT, descriptions TEXT NOT NULL DEFAULT '{}', + category TEXT, version TEXT, content TEXT NOT NULL, + content_md5 TEXT DEFAULT '', metadata TEXT DEFAULT '{}', + commit_msg TEXT, created_by TEXT NOT NULL, source_path TEXT, + created_at DATETIME + )`, + `CREATE TABLE item_tag_dicts ( + id TEXT PRIMARY KEY, slug TEXT NOT NULL UNIQUE, + tag_class TEXT NOT NULL DEFAULT 'custom', created_by TEXT NOT NULL, + created_at DATETIME + )`, + `CREATE TABLE item_tags ( + id TEXT PRIMARY KEY, item_id TEXT NOT NULL, tag_id TEXT NOT NULL, + created_at DATETIME, UNIQUE(item_id, tag_id) + )`, + `CREATE TABLE item_categories ( + id TEXT PRIMARY KEY, slug TEXT NOT NULL UNIQUE, icon TEXT, + sort_order INTEGER DEFAULT 0, names TEXT NOT NULL DEFAULT '{}', + descriptions TEXT DEFAULT '{}', created_by TEXT NOT NULL, + created_at DATETIME, updated_at DATETIME + )`, + `CREATE TABLE scan_jobs ( + id TEXT PRIMARY KEY, item_id TEXT NOT NULL, item_revision INTEGER NOT NULL DEFAULT 0, + trigger_type TEXT NOT NULL, trigger_user TEXT, priority INTEGER NOT NULL DEFAULT 5, + status TEXT NOT NULL DEFAULT 'pending', retry_count INTEGER DEFAULT 0, + max_attempts INTEGER DEFAULT 2, last_error TEXT, scheduled_at DATETIME NOT NULL, + started_at DATETIME, finished_at DATETIME, scan_result_id TEXT, created_at DATETIME + )`, + `CREATE TABLE security_scans ( + id TEXT PRIMARY KEY, item_id TEXT NOT NULL, item_revision INTEGER NOT NULL DEFAULT 0, + trigger_type TEXT NOT NULL, scan_model TEXT, category TEXT DEFAULT '', + builtin_tags TEXT DEFAULT '[]', risk_level TEXT DEFAULT '', + verdict TEXT DEFAULT '', red_flags TEXT DEFAULT '[]', + permissions TEXT DEFAULT '{}', summary TEXT, + recommendations TEXT DEFAULT '[]', raw_output TEXT, + duration_ms INTEGER DEFAULT 0, created_at DATETIME, finished_at DATETIME + )`, + } + for _, s := range stmts { + if err := db.Exec(s).Error; err != nil { + t.Fatalf("create table: %v", err) + } + } + return db +} + +func newIngestService(db *gorm.DB) *CatalogIngestService { + return &CatalogIngestService{ + DB: db, + Parser: &ParserService{}, + TagSvc: &TagService{DB: db}, + CategorySvc: &CategoryService{DB: db}, + ScanJobService: &ScanJobService{DB: db}, + } +} + +// writeSkillBundle materializes a one-entry catalog bundle (a single skill) +// into a temp dir laid out exactly like the upstream bundle: manifest.json, +// index.json, and catalog-download/skills//SKILL.md. The index entry is +// `entry` (with whatever health/evaluation blocks the caller set), and the +// SKILL.md body is `skillBody` so callers can vary the file SHA when they +// want a content change vs. a metadata-only pass. +func writeSkillBundle(t *testing.T, entry catalogEntry, skillBody string) string { + t.Helper() + dir := t.TempDir() + + manifest := map[string]any{ + "schema_version": SupportedBundleSchemaVersion, + "generated_at": "2026-05-25T00:00:00Z", + "entry_count": 1, + "index_sha256": "test-sha", + "type_counts": map[string]int{entry.Type: 1}, + } + mb, _ := json.Marshal(manifest) + if err := os.WriteFile(filepath.Join(dir, "manifest.json"), mb, 0o644); err != nil { + t.Fatalf("write manifest: %v", err) + } + + ib, _ := json.Marshal([]catalogEntry{entry}) + if err := os.WriteFile(filepath.Join(dir, "index.json"), ib, 0o644); err != nil { + t.Fatalf("write index: %v", err) + } + + skillDir := filepath.Join(dir, "catalog-download", "skills", entry.ID) + if err := os.MkdirAll(skillDir, 0o755); err != nil { + t.Fatalf("mkdir skill: %v", err) + } + if err := os.WriteFile(filepath.Join(skillDir, "SKILL.md"), []byte(skillBody), 0o644); err != nil { + t.Fatalf("write SKILL.md: %v", err) + } + return dir +} + +// sampleHealth / sampleEvaluation return the raw upstream JSON shape. Each +// deliberately includes an EXTRA field NOT present in the old fixed structs +// (signals.install_popularity, evaluation_mode) so the tests prove the +// raw-passthrough preserves the whole upstream object end-to-end. +func sampleHealth() json.RawMessage { + return json.RawMessage(`{"score":0.82,"signals":{"freshness":0.9,"popularity":0.7,"source_trust":0.85,"install_popularity":42},"freshness_label":"fresh","last_commit":"2026-05-01T00:00:00Z"}`) +} + +func sampleEvaluation() json.RawMessage { + return json.RawMessage(`{"coding_relevance":4.5,"doc_completeness":4.0,"final_score":4.2,"decision":"accept","model_id":"deepseek-v4","rubric_version":"v2","evaluated_at":"2026-05-10T00:00:00Z","evaluation_mode":"deep"}`) +} + +func loadItemBySlug(t *testing.T, db *gorm.DB, slug string) models.CapabilityItem { + t.Helper() + var item models.CapabilityItem + if err := db.Where("slug = ?", slug).First(&item).Error; err != nil { + t.Fatalf("load item %q: %v", slug, err) + } + return item +} + +// decodeObj unmarshals a stored jsonb column into a generic map so tests can +// assert on arbitrary upstream fields without a fixed struct. +func decodeObj(t *testing.T, raw []byte) map[string]any { + t.Helper() + m := map[string]any{} + if err := json.Unmarshal(raw, &m); err != nil { + t.Fatalf("unmarshal jsonb: %v (raw=%s)", err, string(raw)) + } + return m +} + +// 4.1 — an ingest entry carrying health + evaluation results in the +// capability_items row having the correctly serialized JSON after create, +// and after a subsequent content change (updateItem path). +func TestIngest_HealthEvaluation_PersistedOnCreate(t *testing.T) { + db := newIngestTestDB(t) + svc := newIngestService(db) + + skillBody := "---\nname: Demo Skill\ndescription: does X\n---\n# Demo\nbody v1\n" + entry := catalogEntry{ + ID: "demo-skill", + Type: "skill", + Source: "anthropic/demo", + Description: "does X", + Category: "tooling", + Tags: []string{"demo"}, + FinalScore: 4.2, + Health: sampleHealth(), + Evaluation: sampleEvaluation(), + } + dir := writeSkillBundle(t, entry, skillBody) + + res, err := svc.Ingest(context.Background(), IngestSource{Dir: dir}, IngestOptions{TriggerUser: "tester"}) + if err != nil { + t.Fatalf("ingest: %v", err) + } + if res.Added != 1 { + t.Fatalf("expected Added=1, got Added=%d updated=%d failed=%d errors=%v", res.Added, res.Updated, res.Failed, res.Errors) + } + + item := loadItemBySlug(t, db, "demo-skill") + + gotHealth := decodeObj(t, item.Health) + if gotHealth["score"] != 0.82 || gotHealth["freshness_label"] != "fresh" { + t.Fatalf("health not serialized correctly: %+v (raw=%s)", gotHealth, string(item.Health)) + } + signals, ok := gotHealth["signals"].(map[string]any) + if !ok || signals["popularity"] != 0.7 { + t.Fatalf("health.signals not serialized correctly: %+v (raw=%s)", gotHealth, string(item.Health)) + } + // Extra upstream field must survive the ingest verbatim. + if signals["install_popularity"] != float64(42) { + t.Fatalf("extra health field install_popularity dropped: %+v (raw=%s)", signals, string(item.Health)) + } + + gotEval := decodeObj(t, item.Evaluation) + if gotEval["decision"] != "accept" || gotEval["final_score"] != 4.2 || gotEval["coding_relevance"] != 4.5 { + t.Fatalf("evaluation not serialized correctly: %+v (raw=%s)", gotEval, string(item.Evaluation)) + } + if gotEval["evaluation_mode"] != "deep" { + t.Fatalf("extra evaluation field evaluation_mode dropped: %+v (raw=%s)", gotEval, string(item.Evaluation)) + } + + // Now re-ingest with a changed file body (different SHA) AND mutated + // health/evaluation — exercises the updateItem content-changed path. + entry.Health = json.RawMessage(`{"score":0.5,"freshness_label":"stale","signals":{"freshness":0.4,"popularity":0.3,"source_trust":0.5}}`) + entry.Evaluation = json.RawMessage(`{"final_score":2.0,"decision":"reject","model_id":"deepseek-v4"}`) + dir2 := writeSkillBundle(t, entry, skillBody+"\nbody v2\n") + + res2, err := svc.Ingest(context.Background(), IngestSource{Dir: dir2}, IngestOptions{TriggerUser: "tester"}) + if err != nil { + t.Fatalf("re-ingest (content change): %v", err) + } + if res2.Updated != 1 { + t.Fatalf("expected Updated=1 on content change, got added=%d updated=%d metadataUpdated=%d failed=%d errors=%v", + res2.Added, res2.Updated, res2.MetadataUpdated, res2.Failed, res2.Errors) + } + + item2 := loadItemBySlug(t, db, "demo-skill") + gotHealth2 := decodeObj(t, item2.Health) + if gotHealth2["score"] != 0.5 || gotHealth2["freshness_label"] != "stale" { + t.Fatalf("updateItem did not refresh health: %+v (raw=%s)", gotHealth2, string(item2.Health)) + } + gotEval2 := decodeObj(t, item2.Evaluation) + if gotEval2["decision"] != "reject" { + t.Fatalf("updateItem did not refresh evaluation: %+v (raw=%s)", gotEval2, string(item2.Evaluation)) + } +} + +// 4.2 — an entry WITHOUT health/evaluation does not error and the columns are +// `{}` (empty object), never null. +func TestIngest_NoHealthEvaluation_ColumnsAreEmptyObject(t *testing.T) { + db := newIngestTestDB(t) + svc := newIngestService(db) + + skillBody := "---\nname: Plain Skill\ndescription: no extras\n---\n# Plain\nbody\n" + entry := catalogEntry{ + ID: "plain-skill", + Type: "skill", + Source: "anthropic/plain", + Description: "no extras", + Category: "tooling", + FinalScore: 3.0, + // Health and Evaluation intentionally nil. + } + dir := writeSkillBundle(t, entry, skillBody) + + res, err := svc.Ingest(context.Background(), IngestSource{Dir: dir}, IngestOptions{TriggerUser: "tester"}) + if err != nil { + t.Fatalf("ingest: %v", err) + } + if res.Added != 1 || res.Failed != 0 { + t.Fatalf("expected clean add, got added=%d failed=%d errors=%v", res.Added, res.Failed, res.Errors) + } + + item := loadItemBySlug(t, db, "plain-skill") + if string(item.Health) != "{}" { + t.Fatalf("expected health '{}', got %q", string(item.Health)) + } + if string(item.Evaluation) != "{}" { + t.Fatalf("expected evaluation '{}', got %q", string(item.Evaluation)) + } +} + +// 4.3 — regression for the P1 fix. An existing row with empty {} health/eval +// and an UNCHANGED file SHA + metadata must NOT be skipped when a later bundle +// carries health/evaluation: the metadata-only path detects the jsonb drift +// and backfills the columns. +func TestIngest_MetadataOnlyPath_BackfillsHealthEvaluation(t *testing.T) { + db := newIngestTestDB(t) + svc := newIngestService(db) + + // First pass: no health/evaluation (columns end up '{}'). + skillBody := "---\nname: Backfill Skill\ndescription: stable\n---\n# Backfill\nstable body\n" + entry := catalogEntry{ + ID: "backfill-skill", + Type: "skill", + Source: "anthropic/backfill", + Description: "stable", + Category: "tooling", + FinalScore: 3.5, + } + dir := writeSkillBundle(t, entry, skillBody) + if _, err := svc.Ingest(context.Background(), IngestSource{Dir: dir}, IngestOptions{TriggerUser: "tester"}); err != nil { + t.Fatalf("first ingest: %v", err) + } + before := loadItemBySlug(t, db, "backfill-skill") + if string(before.Health) != "{}" || string(before.Evaluation) != "{}" { + t.Fatalf("setup expected empty health/eval, got health=%q eval=%q", string(before.Health), string(before.Evaluation)) + } + beforeRevision := before.CurrentRevision + + // Second pass: identical file body (same SHA) and identical metadata, + // but now carrying health + evaluation. This MUST route through the + // metadata-only path (file SHA unchanged) and update the columns rather + // than skip the item. + entry.Health = sampleHealth() + entry.Evaluation = sampleEvaluation() + dir2 := writeSkillBundle(t, entry, skillBody) + + res, err := svc.Ingest(context.Background(), IngestSource{Dir: dir2}, IngestOptions{TriggerUser: "tester"}) + if err != nil { + t.Fatalf("second ingest: %v", err) + } + if res.MetadataUpdated != 1 { + t.Fatalf("expected MetadataUpdated=1 (not skipped), got added=%d updated=%d metadataUpdated=%d skipped=%d errors=%v", + res.Added, res.Updated, res.MetadataUpdated, res.Skipped, res.Errors) + } + if res.Skipped != 0 { + t.Fatalf("item must NOT be skipped — health/eval drift should trigger metadata update; skipped=%d", res.Skipped) + } + if res.Updated != 0 { + t.Fatalf("file SHA unchanged so the content path must not fire; updated=%d", res.Updated) + } + + after := loadItemBySlug(t, db, "backfill-skill") + gotHealth := decodeObj(t, after.Health) + if gotHealth["score"] != 0.82 || gotHealth["freshness_label"] != "fresh" { + t.Fatalf("metadata-only path did not backfill health: %+v (raw=%s)", gotHealth, string(after.Health)) + } + // Extra upstream field preserved through the metadata-only backfill path too. + if signals, ok := gotHealth["signals"].(map[string]any); !ok || signals["install_popularity"] != float64(42) { + t.Fatalf("metadata-only backfill dropped extra health field: %+v (raw=%s)", gotHealth, string(after.Health)) + } + gotEval := decodeObj(t, after.Evaluation) + if gotEval["decision"] != "accept" { + t.Fatalf("metadata-only path did not backfill evaluation: %+v (raw=%s)", gotEval, string(after.Evaluation)) + } + if gotEval["evaluation_mode"] != "deep" { + t.Fatalf("metadata-only backfill dropped extra evaluation field: %+v (raw=%s)", gotEval, string(after.Evaluation)) + } + // Metadata-only path must not bump the revision (no new version row). + if after.CurrentRevision != beforeRevision { + t.Fatalf("metadata-only path must not bump revision: before=%d after=%d", beforeRevision, after.CurrentRevision) + } +} + +// rawBlockJSON must enforce the "JSON object" column contract: empty forms, +// invalid JSON, AND valid-but-wrong scalar/array payloads all normalize to +// "{}". A non-empty object passes through (whitespace-compacted) unchanged. +func TestRawBlockJSON_ObjectContract(t *testing.T) { + cases := []struct { + name string + in json.RawMessage + want string + }{ + {"nil", nil, "{}"}, + {"empty bytes", json.RawMessage(``), "{}"}, + {"json null", json.RawMessage(`null`), "{}"}, + {"empty object", json.RawMessage(`{}`), "{}"}, + {"array", json.RawMessage(`[1,2]`), "{}"}, + {"string scalar", json.RawMessage(`"x"`), "{}"}, + {"number scalar", json.RawMessage(`42`), "{}"}, + {"bool scalar", json.RawMessage(`true`), "{}"}, + {"invalid json", json.RawMessage(`{not json`), "{}"}, + {"object passthrough", json.RawMessage(`{ "a" : 1 }`), `{"a":1}`}, + {"object with extra field", json.RawMessage(`{"score":0.5,"extra":[1,2]}`), `{"score":0.5,"extra":[1,2]}`}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := string(rawBlockJSON(tc.in)); got != tc.want { + t.Fatalf("rawBlockJSON(%s) = %q, want %q", string(tc.in), got, tc.want) + } + }) + } +} + +// 4.x — non-object health/evaluation payloads must end up stored as "{}", +// proven end-to-end through the real ingest (not just the unit helper). +func TestIngest_NonObjectHealthEvaluation_StoredAsEmptyObject(t *testing.T) { + db := newIngestTestDB(t) + svc := newIngestService(db) + + skillBody := "---\nname: Wrong Shape\ndescription: bad upstream\n---\n# Wrong\nbody\n" + entry := catalogEntry{ + ID: "wrong-shape", + Type: "skill", + Source: "anthropic/wrong", + Description: "bad upstream", + Category: "tooling", + FinalScore: 2.0, + Health: json.RawMessage(`[1,2]`), + Evaluation: json.RawMessage(`"x"`), + } + dir := writeSkillBundle(t, entry, skillBody) + + res, err := svc.Ingest(context.Background(), IngestSource{Dir: dir}, IngestOptions{TriggerUser: "tester"}) + if err != nil { + t.Fatalf("ingest: %v", err) + } + if res.Added != 1 || res.Failed != 0 { + t.Fatalf("expected clean add, got added=%d failed=%d errors=%v", res.Added, res.Failed, res.Errors) + } + + item := loadItemBySlug(t, db, "wrong-shape") + if string(item.Health) != "{}" { + t.Fatalf("array health must be rejected to '{}', got %q", string(item.Health)) + } + if string(item.Evaluation) != "{}" { + t.Fatalf("scalar evaluation must be rejected to '{}', got %q", string(item.Evaluation)) + } +} diff --git a/server/internal/services/catalog_ingest_service.go b/server/internal/services/catalog_ingest_service.go index 07593520..084b874e 100644 --- a/server/internal/services/catalog_ingest_service.go +++ b/server/internal/services/catalog_ingest_service.go @@ -44,6 +44,7 @@ package services import ( "archive/tar" + "bytes" "compress/gzip" "context" "encoding/json" @@ -53,6 +54,7 @@ import ( "net/http" "os" "path/filepath" + "reflect" "strings" "time" @@ -169,8 +171,14 @@ type catalogBundleManifest struct { } // catalogEntry is the per-entry shape we care about from index.json. -// We deliberately keep this struct small — index.json carries many fields -// (evaluation, freshness_label, weak_dims, …) that the DB does not need. +// The catalog also carries health (freshness/popularity/source_trust signals) +// and evaluation (6 rubric dimensions + final decision) blocks, which we +// persist VERBATIM into dedicated jsonb columns. They are captured as +// json.RawMessage rather than decoded into fixed structs so that any extra +// upstream fields (e.g. health.signals.install_popularity, +// evaluation.evaluation_mode) survive the ingest unmodified — see design.md +// decision 1 ("store the whole upstream JSON shape"). Other top-level fields +// index.json carries (e.g. weak_dims) remain intentionally unmapped. type catalogEntry struct { ID string `json:"id"` Type string `json:"type"` @@ -181,6 +189,8 @@ type catalogEntry struct { Tags []string `json:"tags"` FinalScore float64 `json:"final_score"` Security *catalogSecurityBlock `json:"security,omitempty"` + Health json.RawMessage `json:"health,omitempty"` + Evaluation json.RawMessage `json:"evaluation,omitempty"` } // catalogSecurityBlock mirrors the schema written by the upstream LLM @@ -530,10 +540,53 @@ func (s *CatalogIngestService) computeMetadataDelta(item *models.CapabilityItem, if entry.FinalScore > 0 && item.ExperienceScore != entry.FinalScore { return true } + // health/evaluation JSONB drift: detected so a backfill run (file SHA + + // other metadata unchanged) still routes through the metadata-only path. + if !jsonbObjectEqual(item.Health, healthJSON(entry.Health)) { + return true + } + if !jsonbObjectEqual(item.Evaluation, evaluationJSON(entry.Evaluation)) { + return true + } // Tags: compared in applyMetadataDelta itself (need TagSvc query). return false } +// jsonbObjectEqual compares two jsonb payloads for semantic equality, +// tolerating the pre-migration column states (empty bytes, `null`, `{}`) +// which all count as "empty". Byte comparison alone would be wrong because +// json.Marshal does not guarantee key order, and an empty column may be +// stored as any of those three forms. +func jsonbObjectEqual(a, b datatypes.JSON) bool { + var va, vb any + emptyA := len(a) == 0 + if !emptyA { + _ = json.Unmarshal(a, &va) + emptyA = isEmptyJSONValue(va) + } + emptyB := len(b) == 0 + if !emptyB { + _ = json.Unmarshal(b, &vb) + emptyB = isEmptyJSONValue(vb) + } + if emptyA || emptyB { + return emptyA && emptyB + } + return reflect.DeepEqual(va, vb) +} + +// isEmptyJSONValue reports whether a decoded JSON value is "empty" for the +// purposes of jsonb drift detection: JSON null, or an empty object. +func isEmptyJSONValue(v any) bool { + if v == nil { + return true + } + if m, ok := v.(map[string]any); ok { + return len(m) == 0 + } + return false +} + // descriptionsJSONEqual compares two locale → text JSON maps for semantic // equality. Byte comparison would be incorrect because json.Marshal does // not guarantee key order across Go versions. @@ -577,6 +630,11 @@ func (s *CatalogIngestService) applyMetadataDelta(item *models.CapabilityItem, e if entry.FinalScore > 0 { updates["experience_score"] = entry.FinalScore } + // health/evaluation are always written on this path so a metadata-only + // upstream change (no primary-file diff) still backfills these columns. + // Nil blocks normalize to an empty object, so this never clobbers with null. + updates["health"] = healthJSON(entry.Health) + updates["evaluation"] = evaluationJSON(entry.Evaluation) if len(updates) > 0 { if err := s.DB.Model(&models.CapabilityItem{}).Where("id = ?", item.ID).Updates(updates).Error; err != nil { return err @@ -641,6 +699,8 @@ func (s *CatalogIngestService) updateItem( existing.Content = parsed.Content existing.Source = source existing.ExperienceScore = experienceScore + existing.Health = healthJSON(entry.Health) + existing.Evaluation = evaluationJSON(entry.Evaluation) existing.Status = "active" existing.Metadata = metadataJSON(meta) existing.SourcePath = primaryPath @@ -732,6 +792,8 @@ func (s *CatalogIngestService) insertItem( Version: parsed.Version, Content: parsed.Content, Metadata: metadataJSON(meta), + Health: healthJSON(entry.Health), + Evaluation: evaluationJSON(entry.Evaluation), SourcePath: primaryPath, SourceSHA: fileSHA, Source: source, @@ -1077,6 +1139,53 @@ func buildDescriptionsJSON(entry catalogEntry) datatypes.JSON { return datatypes.JSON(b) } +// healthJSON normalizes the upstream health block into a jsonb payload, +// preserving the raw bytes verbatim (see catalogEntry doc / design.md +// decision 1). Empty/nil/null/empty-object input collapses to "{}" so the +// column always holds a valid, non-null JSON object. +func healthJSON(raw json.RawMessage) datatypes.JSON { + return rawBlockJSON(raw) +} + +// evaluationJSON mirrors healthJSON for the evaluation block. Extra upstream +// fields (e.g. evaluation_mode) and missing rubric dimensions are preserved +// exactly as upstream emitted them — no field is dropped or defaulted. +func evaluationJSON(raw json.RawMessage) datatypes.JSON { + return rawBlockJSON(raw) +} + +// rawBlockJSON passes an upstream jsonb block through unchanged (lossless), +// only normalizing to a canonical "{}" when the payload is not a usable +// object. It compacts whitespace so byte-identical re-ingests stay stable, but +// never reshapes or drops keys of a real object. +// +// The column contract is "JSON object" (see the swagger `object` type on +// CapabilityItem.Health/Evaluation). So anything that isn't a NON-EMPTY JSON +// object — empty bytes, invalid JSON, null, empty object, OR a valid-but-wrong +// scalar/array (e.g. `health: []`, `evaluation: "x"`) — collapses to "{}" +// rather than persisting a malformed shape the frontend can't consume. +func rawBlockJSON(raw json.RawMessage) datatypes.JSON { + empty := datatypes.JSON([]byte("{}")) + if len(raw) == 0 { + return empty + } + var v any + if err := json.Unmarshal(raw, &v); err != nil { + return empty + } + // Enforce the object contract: only a non-empty JSON object passes through. + obj, ok := v.(map[string]any) + if !ok || len(obj) == 0 { + return empty + } + var buf bytes.Buffer + if err := json.Compact(&buf, raw); err != nil { + // Already validated above; fall back to the raw bytes rather than dropping data. + return datatypes.JSON([]byte(raw)) + } + return datatypes.JSON(buf.Bytes()) +} + // chooseTags implements the precedence rule: upstream tags win when // non-empty (catalog has authoritative taxonomy), otherwise fall back to // what the per-file parser extracted (e.g. SKILL.md frontmatter). @@ -1322,4 +1431,3 @@ func readIndex(bundleDir string) ([]catalogEntry, error) { } return entries, nil } - diff --git a/server/internal/services/scan_service_test.go b/server/internal/services/scan_service_test.go index 4ef8e80f..7fae7d63 100644 --- a/server/internal/services/scan_service_test.go +++ b/server/internal/services/scan_service_test.go @@ -31,7 +31,7 @@ func TestScanItem_PluginSkip(t *testing.T) { slug TEXT NOT NULL, item_type TEXT NOT NULL, name TEXT NOT NULL, description TEXT, descriptions TEXT NOT NULL DEFAULT '{}', category TEXT, version TEXT DEFAULT '1.0.0', content TEXT, content_md5 TEXT DEFAULT '', current_revision INTEGER NOT NULL DEFAULT 1, - metadata TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, + metadata TEXT DEFAULT '{}', health TEXT DEFAULT '{}', evaluation TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, source_type TEXT DEFAULT 'direct', source TEXT DEFAULT '', preview_count INTEGER DEFAULT 0, install_count INTEGER DEFAULT 0, favorite_count INTEGER DEFAULT 0, status TEXT DEFAULT 'active', @@ -140,6 +140,8 @@ func TestScanItemUpdatesCategoryFromScanResult(t *testing.T) { content_md5 TEXT DEFAULT '', current_revision INTEGER NOT NULL DEFAULT 1, metadata TEXT DEFAULT '{}', + health TEXT DEFAULT '{}', + evaluation TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, source_type TEXT DEFAULT 'direct', @@ -327,6 +329,8 @@ func TestScanItemBackfillsBuiltinTagsFromScanResult(t *testing.T) { content_md5 TEXT DEFAULT '', current_revision INTEGER NOT NULL DEFAULT 1, metadata TEXT DEFAULT '{}', + health TEXT DEFAULT '{}', + evaluation TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, source_type TEXT DEFAULT 'direct', @@ -526,6 +530,8 @@ func newScanShortCircuitTestDB(t *testing.T) *gorm.DB { content_md5 TEXT DEFAULT '', current_revision INTEGER NOT NULL DEFAULT 1, metadata TEXT DEFAULT '{}', + health TEXT DEFAULT '{}', + evaluation TEXT DEFAULT '{}', source_path TEXT, source_sha TEXT, source_type TEXT DEFAULT 'direct',