From 91fb07b7e566623736a27706a48f739000b58362 Mon Sep 17 00:00:00 2001 From: Jeffrey Smith Date: Mon, 30 Mar 2026 18:39:24 +0000 Subject: [PATCH] Add package update/export tests and mark v0.5.4 done in roadmap 14 new tests: version bump, rejection cases (same/older/core/type/id mismatch), schema add column/table with data preservation, settings merge, export with valid zip, default allowlist, wildcard allowlist. Existing bundled tests updated for curated default behavior. Co-Authored-By: Claude Opus 4.6 (1M context) --- ROADMAP.md | 15 +- server/handlers/packages_bundled_test.go | 6 +- server/handlers/packages_update_test.go | 494 +++++++++++++++++++++++ 3 files changed, 505 insertions(+), 10 deletions(-) create mode 100644 server/handlers/packages_update_test.go diff --git a/ROADMAP.md b/ROADMAP.md index 79a75f1..0f1464d 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -389,16 +389,17 @@ participation is post-MVP. | "Requires Review" filter | Dropdown filter option or dedicated button for `pending_review` packages. Useful when the package list grows long. | | Extreme UI scale polish | At 150%+ scale: user menu items overflow viewport (need scroll), topbar elements clipped. Ensure all surfaces and menus are usable at scale extremes (80%–200%). | -### v0.5.4 — Package Updates (planned) +### v0.5.4 — Package Updates | Step | Status | Description | |------|--------|-------------| -| Version comparison | | Compare installed package version against incoming `.pkg` version. Semantic version parsing. Skip if same or older. | -| Schema migration on update | | Diff declared `db_tables` against existing ext_data schema. Add new columns, add new tables. No destructive changes (no column drops, no type changes). | -| Data preservation | | ext_data tables survive update. Only additive schema changes applied. Settings merged (new keys added, existing preserved). | -| Update API | | `PUT /api/v1/packages/:id/update` — accepts `.pkg` archive, validates version bump, applies schema diff, replaces code (JS/CSS/Starlark). | -| Admin UI | | Update button on package card when newer version available. Version comparison badge. Update confirmation via `sw.ui.Confirm`. | -| Rollback story | | Export package before update (existing export). Document manual rollback: re-install old `.pkg`. No automatic rollback — KISS. | +| Curate bundled packages | ✅ | Default set reduced from 23 to 8 core packages. `BUNDLED_PACKAGES=*` for all; explicit list for custom. | +| Version comparison | ✅ | Semver parsing (`ParseSemver`) with prerelease support. Version bump validated on update. | +| Schema migration on update | ✅ | `MigrateExtTables` diffs `db_tables` against existing schema. Adds new columns/tables. No destructive changes. | +| Data preservation | ✅ | ext_data tables survive update. Settings merged (new keys added with defaults, existing preserved). | +| Update API | ✅ | `POST /api/v1/admin/packages/:id/update` — accepts `.pkg` archive, validates version bump, applies schema diff, replaces code. | +| Admin UI | ✅ | Update button on non-core package cards. Confirm dialog before upload. | +| Export API | ✅ | `GET /api/v1/admin/packages/:id/export` — streams installed package as .pkg ZIP. Manual rollback: export → update → re-install old .pkg if needed. | ### v0.5.5 — Upgrade Testing (planned) diff --git a/server/handlers/packages_bundled_test.go b/server/handlers/packages_bundled_test.go index 170ef80..8fcb284 100644 --- a/server/handlers/packages_bundled_test.go +++ b/server/handlers/packages_bundled_test.go @@ -88,7 +88,7 @@ func TestBundledInstall_FreshDB(t *testing.T) { "route": "/s/test-b", }) - InstallBundledPackages(bundledDir, packagesDir, "", stores, nil) + InstallBundledPackages(bundledDir, packagesDir, "*", stores, nil) // Both packages should be installed and enabled for _, id := range []string{"test-surface-a", "test-surface-b"} { @@ -127,7 +127,7 @@ func TestBundledInstall_SkipsExisting(t *testing.T) { "type": "surface", }) - InstallBundledPackages(bundledDir, packagesDir, "", stores, nil) + InstallBundledPackages(bundledDir, packagesDir, "*", stores, nil) // Package should retain original title (not overwritten) pkg, err := stores.Packages.Get(ctx, "pre-existing") @@ -174,7 +174,7 @@ func TestBundledInstall_DormantPackage(t *testing.T) { "requires": []string{"chat"}, }) - InstallBundledPackages(bundledDir, packagesDir, "", stores, nil) + InstallBundledPackages(bundledDir, packagesDir, "*", stores, nil) pkg, err := stores.Packages.Get(ctx, "dormant-pkg") if err != nil || pkg == nil { diff --git a/server/handlers/packages_update_test.go b/server/handlers/packages_update_test.go new file mode 100644 index 0000000..933732a --- /dev/null +++ b/server/handlers/packages_update_test.go @@ -0,0 +1,494 @@ +package handlers + +import ( + "archive/zip" + "bytes" + "context" + "database/sql" + "encoding/json" + "fmt" + "io" + "mime/multipart" + "net/http" + "net/http/httptest" + "testing" + + "github.com/gin-gonic/gin" + + "switchboard-core/database" + "switchboard-core/store" +) + +// ── Helpers ──────────────────────────────────── + +// buildPkgBytes creates a .pkg archive in memory and returns the ZIP bytes. +func buildPkgBytes(t *testing.T, manifest map[string]any) []byte { + t.Helper() + var buf bytes.Buffer + zw := zip.NewWriter(&buf) + data, _ := json.Marshal(manifest) + w, _ := zw.Create("manifest.json") + w.Write(data) + zw.Close() + return buf.Bytes() +} + +// seedPackage installs a package directly into the DB for testing. +func seedPackage(t *testing.T, stores store.Stores, manifest map[string]any) { + t.Helper() + ctx := context.Background() + id := manifest["id"].(string) + title := manifest["title"].(string) + source := "extension" + if s, ok := manifest["source"].(string); ok { + source = s + } + if err := stores.Packages.Seed(ctx, id, title, source, manifest); err != nil { + t.Fatal(err) + } + version, _ := manifest["version"].(string) + if version == "" { + version = "0.0.0" + } + pkgType, _ := manifest["type"].(string) + if pkgType == "" { + pkgType = "surface" + } + tier, _ := manifest["tier"].(string) + if tier == "" { + tier = "browser" + } + pkg := &store.PackageRegistration{ + Title: title, + Type: pkgType, + Version: version, + Tier: tier, + Enabled: true, + Manifest: manifest, + } + if err := stores.Packages.Update(ctx, id, pkg); err != nil { + t.Fatalf("seedPackage Update failed for %s: %v", id, err) + } + // Verify version was actually stored + check, _ := stores.Packages.Get(ctx, id) + if check != nil && check.Version != version { + t.Fatalf("seedPackage: version not stored correctly for %s: got %q want %q", id, check.Version, version) + } +} + +// setupUpdateRouter creates a gin router with the update and export routes. +func setupUpdateRouter(t *testing.T, stores store.Stores) *gin.Engine { + t.Helper() + gin.SetMode(gin.TestMode) + r := gin.New() + handler := NewPackageHandler(stores) + r.POST("/api/v1/admin/packages/:id/update", handler.UpdatePackage) + r.GET("/api/v1/admin/packages/:id/export", handler.ExportPackage) + return r +} + +// doUpdate performs a multipart upload to the update endpoint. +func doUpdate(router *gin.Engine, pkgID string, pkgBytes []byte) *httptest.ResponseRecorder { + body := &bytes.Buffer{} + writer := multipart.NewWriter(body) + part, _ := writer.CreateFormFile("file", "test.pkg") + part.Write(pkgBytes) + writer.Close() + + req := httptest.NewRequest("POST", "/api/v1/admin/packages/"+pkgID+"/update", body) + req.Header.Set("Content-Type", writer.FormDataContentType()) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + return w +} + +// ── Tests ──────────────────────────────────── + +func TestUpdatePackage_VersionBump(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + seedPackage(t, stores, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.1.0", + }) + + w := doUpdate(router, "test-pkg", pkg) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + // Verify version updated + updated, _ := stores.Packages.Get(ctx, "test-pkg") + if updated.Version != "1.1.0" { + t.Errorf("version = %q, want %q", updated.Version, "1.1.0") + } + + // Check response body + var resp map[string]any + json.Unmarshal(w.Body.Bytes(), &resp) + if resp["version"] != "1.1.0" { + t.Errorf("response version = %v, want 1.1.0", resp["version"]) + } + if resp["previous_version"] != "1.0.0" { + t.Errorf("response previous_version = %v, want 1.0.0", resp["previous_version"]) + } +} + +func TestUpdatePackage_SameVersion_Rejected(t *testing.T) { + stores := newTestStores(t) + + seedPackage(t, stores, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + w := doUpdate(router, "test-pkg", pkg) + if w.Code != http.StatusConflict { + t.Fatalf("expected 409, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_OlderVersion_Rejected(t *testing.T) { + stores := newTestStores(t) + + seedPackage(t, stores, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "2.0.0", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + w := doUpdate(router, "test-pkg", pkg) + if w.Code != http.StatusConflict { + t.Fatalf("expected 409, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_CorePackage_Rejected(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + // Seed as core package + stores.Packages.Seed(ctx, "core-pkg", "Core", "core", map[string]any{ + "id": "core-pkg", "title": "Core", "type": "surface", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "core-pkg", "title": "Core", "type": "surface", "version": "2.0.0", + }) + + w := doUpdate(router, "core-pkg", pkg) + if w.Code != http.StatusForbidden { + t.Fatalf("expected 403, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_TypeMismatch_Rejected(t *testing.T) { + stores := newTestStores(t) + + seedPackage(t, stores, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "extension", "version": "2.0.0", + "tools": []any{map[string]any{"name": "test"}}, + }) + + w := doUpdate(router, "test-pkg", pkg) + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_IDMismatch_Rejected(t *testing.T) { + stores := newTestStores(t) + + seedPackage(t, stores, map[string]any{ + "id": "test-pkg", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "other-pkg", "title": "Other", "type": "surface", "version": "2.0.0", + }) + + w := doUpdate(router, "test-pkg", pkg) + if w.Code != http.StatusBadRequest { + t.Fatalf("expected 400, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_NotFound(t *testing.T) { + stores := newTestStores(t) + + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "nonexistent", "title": "Test", "type": "surface", "version": "1.0.0", + }) + + w := doUpdate(router, "nonexistent", pkg) + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d: %s", w.Code, w.Body.String()) + } +} + +func TestUpdatePackage_SchemaAddColumn(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + // Install v1 with a table that has one column + seedPackage(t, stores, map[string]any{ + "id": "schema-pkg", "title": "Schema Test", "type": "surface", "version": "1.0.0", + "db_tables": map[string]any{ + "items": map[string]any{ + "columns": map[string]any{"name": "text"}, + }, + }, + }) + // Actually create the table + tables, _ := ParseDBTables(map[string]any{ + "db_tables": map[string]any{ + "items": map[string]any{ + "columns": map[string]any{"name": "text"}, + }, + }, + }) + CreateExtTables(ctx, database.TestDB, database.IsPostgres(), stores, "schema-pkg", tables) + + // Insert a row to verify data preservation + physical := extPhysicalTable("schema-pkg", "items") + _, err := database.TestDB.ExecContext(ctx, + fmt.Sprintf("INSERT INTO %s (id, name) VALUES ('row1', 'hello')", physical)) + if err != nil { + t.Fatal(err) + } + + // Update to v2 with an added column + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "schema-pkg", "title": "Schema Test", "type": "surface", "version": "2.0.0", + "db_tables": map[string]any{ + "items": map[string]any{ + "columns": map[string]any{"name": "text", "priority": "int"}, + }, + }, + }) + + w := doUpdate(router, "schema-pkg", pkg) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + // Verify new column exists and old data preserved + var name string + var priority sql.NullInt64 + err = database.TestDB.QueryRowContext(ctx, + fmt.Sprintf("SELECT name, priority FROM %s WHERE id = 'row1'", physical)). + Scan(&name, &priority) + if err != nil { + t.Fatal("query failed after schema migration:", err) + } + if name != "hello" { + t.Errorf("name = %q, want %q (data should be preserved)", name, "hello") + } +} + +func TestUpdatePackage_SchemaAddTable(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + // Install v1 with no tables + seedPackage(t, stores, map[string]any{ + "id": "schema-pkg-b", "title": "Schema B", "type": "surface", "version": "1.0.0", + }) + + // Update to v2 with a new table + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "schema-pkg-b", "title": "Schema B", "type": "surface", "version": "2.0.0", + "db_tables": map[string]any{ + "audit": map[string]any{ + "columns": map[string]any{"action": "text", "actor": "text"}, + }, + }, + }) + + w := doUpdate(router, "schema-pkg-b", pkg) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + // Verify table was created + physical := extPhysicalTable("schema-pkg-b", "audit") + _, err := database.TestDB.ExecContext(ctx, + fmt.Sprintf("INSERT INTO %s (id, action, actor) VALUES ('row1', 'test', 'user1')", physical)) + if err != nil { + t.Fatal("new table should exist after update:", err) + } +} + +func TestUpdatePackage_SettingsMerge(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + // Install with initial settings + seedPackage(t, stores, map[string]any{ + "id": "settings-pkg", "title": "Settings Test", "type": "surface", "version": "1.0.0", + "settings": []any{ + map[string]any{"key": "color", "type": "text", "default": "blue"}, + }, + }) + // Set existing settings value + existing := json.RawMessage(`{"color":"red"}`) + stores.Packages.SetPackageSettings(ctx, "settings-pkg", existing) + + // Update with new settings key + router := setupUpdateRouter(t, stores) + pkg := buildPkgBytes(t, map[string]any{ + "id": "settings-pkg", "title": "Settings Test", "type": "surface", "version": "2.0.0", + "settings": []any{ + map[string]any{"key": "color", "type": "text", "default": "blue"}, + map[string]any{"key": "size", "type": "int", "default": float64(12)}, + }, + }) + + w := doUpdate(router, "settings-pkg", pkg) + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + // Verify settings: color preserved as "red", size added with default 12 + raw, _ := stores.Packages.GetPackageSettings(ctx, "settings-pkg") + var settings map[string]any + json.Unmarshal(raw, &settings) + + if settings["color"] != "red" { + t.Errorf("color = %v, want %q (existing should be preserved)", settings["color"], "red") + } + if settings["size"] != float64(12) { + t.Errorf("size = %v, want %v (new key should get default)", settings["size"], 12) + } +} + +func TestExportPackage_Basic(t *testing.T) { + stores := newTestStores(t) + + seedPackage(t, stores, map[string]any{ + "id": "export-pkg", "title": "Export Test", "type": "surface", "version": "1.0.0", + }) + + router := setupUpdateRouter(t, stores) + + req := httptest.NewRequest("GET", "/api/v1/admin/packages/export-pkg/export", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Fatalf("expected 200, got %d: %s", w.Code, w.Body.String()) + } + + // Verify response is a valid zip with manifest.json + zr, err := zip.NewReader(bytes.NewReader(w.Body.Bytes()), int64(w.Body.Len())) + if err != nil { + t.Fatal("response is not a valid zip:", err) + } + + foundManifest := false + for _, f := range zr.File { + if f.Name == "manifest.json" { + foundManifest = true + rc, _ := f.Open() + data, _ := io.ReadAll(rc) + rc.Close() + var m map[string]any + if json.Unmarshal(data, &m) != nil { + t.Fatal("manifest.json is not valid JSON") + } + if m["id"] != "export-pkg" { + t.Errorf("manifest id = %v, want export-pkg", m["id"]) + } + } + } + if !foundManifest { + t.Error("zip does not contain manifest.json") + } +} + +func TestExportPackage_NotFound(t *testing.T) { + stores := newTestStores(t) + router := setupUpdateRouter(t, stores) + + req := httptest.NewRequest("GET", "/api/v1/admin/packages/nonexistent/export", nil) + w := httptest.NewRecorder() + router.ServeHTTP(w, req) + + if w.Code != http.StatusNotFound { + t.Fatalf("expected 404, got %d", w.Code) + } +} + +// TestBundledInstall_DefaultAllowlist verifies the curated default set behavior. +func TestBundledInstall_DefaultAllowlist(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + bundledDir := t.TempDir() + packagesDir := t.TempDir() + + // Create packages: one in default set, one not + buildTestPkg(t, bundledDir, map[string]any{ + "id": "notes", "title": "Notes", "type": "surface", "version": "1.0.0", + }) + buildTestPkg(t, bundledDir, map[string]any{ + "id": "regex-tester", "title": "Regex Tester", "type": "extension", "version": "1.0.0", + }) + + // Empty allowlist → curated defaults + InstallBundledPackages(bundledDir, packagesDir, "", stores, nil) + + // "notes" is in the default set → should be installed + pkg, err := stores.Packages.Get(ctx, "notes") + if err != nil || pkg == nil { + t.Fatal("notes should be installed (in default set)") + } + + // "regex-tester" is NOT in the default set → should be filtered + pkg, _ = stores.Packages.Get(ctx, "regex-tester") + if pkg != nil { + t.Fatal("regex-tester should NOT be installed (not in default set)") + } +} + +// TestBundledInstall_WildcardAllowlist verifies "*" installs everything. +func TestBundledInstall_WildcardAllowlist(t *testing.T) { + stores := newTestStores(t) + ctx := context.Background() + + bundledDir := t.TempDir() + packagesDir := t.TempDir() + + buildTestPkg(t, bundledDir, map[string]any{ + "id": "any-pkg", "title": "Any", "type": "surface", "version": "1.0.0", + }) + + InstallBundledPackages(bundledDir, packagesDir, "*", stores, nil) + + pkg, err := stores.Packages.Get(ctx, "any-pkg") + if err != nil || pkg == nil { + t.Fatal("any-pkg should be installed with * wildcard") + } +}