diff --git a/CHANGELOG.md b/CHANGELOG.md index 3349f30..8143bff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,32 @@ All notable changes to Armature are documented here. +## v0.7.6 — Code Hygiene + Test Coverage + +**Critical Fixes** +- Removed `channels` from `allowedViews` in `db_module.go` — `ext_view_channels` does not exist; `db.view("channels")` would crash +- Fixed 5 dead API routes in `workflow.html` — rewired to public workflow API (`/api/v1/public/workflows/`) +- Fixed `RenderWorkflow` handler to pass route `:id` param as entry token (was reading unset `channel_id`) + +**Dead Code Removal** +- Deleted `SeedTestChannel()` from `database/testhelper.go` (inserted into nonexistent channels table) +- Removed `RunContext.ChannelID` from `sandbox/runner.go` (vestigial, unused) +- Removed `webhook.Payload.ChannelID` field (channels no longer exist — breaking webhook JSON change) +- Fixed stale comments in `storage.go`, `prometheus.go`, `workflow_module.go` referencing dead `/channels` paths + +**Migration Hygiene** +- Added SQLite placeholder `013_cluster_registry.sql` (PG-only migration, aligns numbering) +- Renumbered SQLite `013_test_runner_type.sql` → `014` to match PG (compat rename in `migrate.go`) +- Documented missing migration 008 in both 009 files (merged into 007 during pre-1.0 consolidation) + +**Test Coverage** +- 82 new workflow routing tests (`routing_test.go`): `ResolveNextStage`, `ResolveStageByName`, `ParseStageConfig`, `evaluateCondition` with all 10 operators +- 10 new middleware tests (`permissions_test.go`): `RequirePermission`, `RequireAdmin`, permission caching, `RateLimiter` (allow/deny/fail-open) + +**Bug Fixes** +- Removed dead Admin "Storage" tab from System category (backend endpoint preserved for future Monitoring use) +- Fixed backup download: `sw.auth.token()` → `sw.auth._getToken()` — both "Download Backup" and server backup download now work + ## v0.7.5 — Headless E2E + CI Gate **CI Gating Redesign** diff --git a/ROADMAP.md b/ROADMAP.md index 38129c9..734508b 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -1,6 +1,6 @@ # Armature — Roadmap -## Current: v0.7.5 — Headless E2E + CI Gate +## Current: v0.7.6 — Code Hygiene + Test Coverage Self-hosted extensible platform kernel. Auth, identity, packages, Starlark sandbox, storage, realtime, and ops are kernel primitives. Everything else @@ -200,33 +200,40 @@ a test coverage gap in the store layer. Fix before v0.8.x kernel expansion. | Step | Status | Description | |------|--------|-------------| -| Remove channels from allowedViews | | `db_module.go` — `ext_view_channels` does not exist. `db.view("channels")` crashes. | -| Fix workflow.html dead routes | | Template calls `/api/v1/channels/{id}/workflow/*` — routes moved to `/api/v1/public/workflows/`. Update template to match. | +| Remove channels from allowedViews | done | `db_module.go` — `ext_view_channels` does not exist. `db.view("channels")` crashes. | +| Fix workflow.html dead routes | done | Template calls `/api/v1/channels/{id}/workflow/*` — routes moved to `/api/v1/public/workflows/`. Update template to match. Also fixed `RenderWorkflow` handler to use route param as entry token. | **Dead Code Removal** | Step | Status | Description | |------|--------|-------------| -| Delete SeedTestChannel() | | `database/testhelper.go` — inserts into nonexistent channels table. | -| Remove RunContext.ChannelID | | `sandbox/runner.go` — vestigial field, unused by any module. | -| Remove webhook.ChannelID | | `webhook/webhook.go` — vestigial struct field from chat era. | -| Fix stale comments | | `storage.go` key format, `prometheus.go` route pattern — reference dead `/channels` paths. | +| Delete SeedTestChannel() | done | `database/testhelper.go` — inserts into nonexistent channels table. | +| Remove RunContext.ChannelID | done | `sandbox/runner.go` — vestigial field, unused by any module. | +| Remove webhook.ChannelID | done | `webhook/webhook.go` — vestigial struct field from chat era. | +| Fix stale comments | done | `storage.go` key format, `prometheus.go` route pattern, `workflow_module.go` param name — reference dead `/channels` paths. | **Migration Hygiene** | Step | Status | Description | |------|--------|-------------| -| Align SQLite migration numbering | | SQLite 013 = test_runner_type, PG 014 = same. Add placeholder `013_cluster_registry.sql` to SQLite (comment-only, explains PG-only). | -| Document missing 008 | | Add comment in both 009 files explaining the gap (merged/removed). | +| Align SQLite migration numbering | done | SQLite 013 placeholder + renumber test_runner_type to 014. Compat rename in migrate.go. | +| Document missing 008 | done | Comment in both 009 files explaining the gap (merged into 007). | **Test Gap Closure** | Step | Status | Description | |------|--------|-------------| -| store/ unit tests | | Direct tests for PG + SQLite store implementations. Priority: packages, ext_data, workflows. Target: ≥30% SLOC ratio (from 2%). | -| workflow/ engine unit tests | | Direct tests for `engine.go`, `routing.go`, `automated.go`. Currently 0% — tested only through handler integration. | -| middleware/ unit tests | | Auth middleware, rate limit, permissions. Currently 8.9%. | -| InstallPackage decomposition | | 224 cognitive complexity. Split into: validate → create → DDL → permissions → triggers. Each independently testable. | +| store/ unit tests | | Deferred to v0.7.8. Direct tests for PG + SQLite store implementations. Requires live DB. Target: ≥30% SLOC ratio. | +| workflow/ routing unit tests | done | 82 tests: ResolveNextStage, ResolveStageByName, ParseStageConfig, evaluateCondition (all operators). | +| middleware/ unit tests | done | 10 tests: RequirePermission, RequireAdmin, permission caching, RateLimiter (allow/deny/fail-open). | +| InstallPackage decomposition | | Deferred to v0.7.8. 224 cognitive complexity. Split into: validate → create → DDL → permissions → triggers. | + +**Bug Fixes** + +| Step | Status | Description | +|------|--------|-------------| +| Remove Admin Storage tab | done | Dead weight under System — backend endpoint preserved for future Monitoring use. | +| Fix backup download | done | `sw.auth.token()` → `sw.auth._getToken()`. Both "Download Backup" and server backup download buttons now work. | ### v0.7.7 — API Tokens + Extension Permissions @@ -261,6 +268,16 @@ sidecar auth (v0.10.x). | `gate_permission` manifest field | | Optional. If set, `ext_api.go` checks this permission before calling `on_request`. Extension doesn't execute for unauthorized users. | | `req["permissions"]` in request dict | | `ext_api.go` resolves user permissions and includes them in the request dict. Extensions can check inline without the module. | +### v0.7.8 — Deferred Test Coverage + +Remaining test gap items deferred from v0.7.6. + +| Step | Status | Description | +|------|--------|-------------| +| store/ unit tests | | Direct tests for PG + SQLite store implementations. Priority: packages, ext_data, workflows. Target: ≥30% SLOC ratio. | +| InstallPackage decomposition | | 224 cognitive complexity. Split into: validate → create → DDL → permissions → triggers. Each independently testable. | +| workflow/ engine integration tests | | Engine lifecycle tests with mock stores (Start, Advance, Cancel, signoff gate). | + --- ## Planned diff --git a/VERSION b/VERSION index 8bd6ba8..c006218 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.7.5 +0.7.6 diff --git a/server/database/migrate.go b/server/database/migrate.go index 6c92aac..a2b290b 100644 --- a/server/database/migrate.go +++ b/server/database/migrate.go @@ -74,6 +74,12 @@ func Migrate() error { } } + // v0.7.6: SQLite 013_test_runner_type.sql was renumbered to 014 to + // align with PG migration numbering. Mark as applied if the old name exists. + if !IsPostgres() { + DB.Exec("UPDATE schema_migrations SET version = '014_test_runner_type.sql' WHERE version = '013_test_runner_type.sql'") + } + // Apply pending migrations. applied := 0 for _, file := range files { diff --git a/server/database/migrations/postgres/009_ha.sql b/server/database/migrations/postgres/009_ha.sql index c62febf..c280ec6 100644 --- a/server/database/migrations/postgres/009_ha.sql +++ b/server/database/migrations/postgres/009_ha.sql @@ -1,6 +1,7 @@ -- ========================================== -- Armature — 009 Multi-Replica HA -- ========================================== +-- Note: migration 008 was merged into 007 during pre-1.0 schema consolidation. CREATE TABLE IF NOT EXISTS ws_tickets ( id TEXT PRIMARY KEY, diff --git a/server/database/migrations/sqlite/009_ha.sql b/server/database/migrations/sqlite/009_ha.sql index c73776f..a2bb390 100644 --- a/server/database/migrations/sqlite/009_ha.sql +++ b/server/database/migrations/sqlite/009_ha.sql @@ -1,6 +1,7 @@ -- ========================================== -- Armature — 009 Multi-Replica HA (SQLite) -- ========================================== +-- Note: migration 008 was merged into 007 during pre-1.0 schema consolidation. CREATE TABLE IF NOT EXISTS ws_tickets ( id TEXT PRIMARY KEY, diff --git a/server/database/migrations/sqlite/013_cluster_registry.sql b/server/database/migrations/sqlite/013_cluster_registry.sql new file mode 100644 index 0000000..fbf91e7 --- /dev/null +++ b/server/database/migrations/sqlite/013_cluster_registry.sql @@ -0,0 +1,7 @@ +-- 013_cluster_registry.sql — placeholder +-- Postgres-only migration: creates UNLOGGED node_registry table for +-- cluster self-assembly (v0.6.0). SQLite deployments are single-node +-- and do not use the cluster registry. This placeholder aligns the +-- migration numbering between dialects. +-- +-- See postgres/013_cluster_registry.sql for the actual schema. diff --git a/server/database/migrations/sqlite/013_test_runner_type.sql b/server/database/migrations/sqlite/014_test_runner_type.sql similarity index 97% rename from server/database/migrations/sqlite/013_test_runner_type.sql rename to server/database/migrations/sqlite/014_test_runner_type.sql index 85c9d5f..1df8130 100644 --- a/server/database/migrations/sqlite/013_test_runner_type.sql +++ b/server/database/migrations/sqlite/014_test_runner_type.sql @@ -1,4 +1,4 @@ --- 013_test_runner_type.sql — v0.7.1 +-- 014_test_runner_type.sql — v0.7.1 -- Adds 'test-runner' to the packages.type CHECK constraint. -- SQLite doesn't support ALTER CHECK — must recreate the table. diff --git a/server/database/testhelper.go b/server/database/testhelper.go index cc5eab1..5c25e36 100644 --- a/server/database/testhelper.go +++ b/server/database/testhelper.go @@ -427,25 +427,6 @@ func SeedAdminsGroupMember(t *testing.T, userID string) { } } -// SeedTestChannel creates a test channel owned by userID and returns the channel ID. -func SeedTestChannel(t *testing.T, userID, title string) string { - t.Helper() - if IsSQLite() { - id := uuid.New().String() - _, err := DB.Exec(`INSERT INTO channels (id, user_id, title) VALUES (?, ?, ?)`, id, userID, title) - if err != nil { - t.Fatalf("SeedTestChannel: %v", err) - } - return id - } - var id string - err := DB.QueryRow(`INSERT INTO channels (user_id, title) VALUES ($1, $2) RETURNING id`, userID, title).Scan(&id) - if err != nil { - t.Fatalf("SeedTestChannel: %v", err) - } - return id -} - // SeedTestTeam creates a test team and returns the team ID. func SeedTestTeam(t *testing.T, name, createdBy string) string { t.Helper() diff --git a/server/middleware/permissions_test.go b/server/middleware/permissions_test.go new file mode 100644 index 0000000..0a58d72 --- /dev/null +++ b/server/middleware/permissions_test.go @@ -0,0 +1,255 @@ +package middleware + +import ( + "context" + "encoding/json" + "errors" + "net/http" + "net/http/httptest" + "testing" + "time" + + "github.com/gin-gonic/gin" + + "armature/auth" + "armature/store" +) + +// --------------------------------------------------------------------------- +// Mocks +// --------------------------------------------------------------------------- + +// mockRateLimitStore implements store.RateLimitStore for testing. +type mockRateLimitStore struct { + allowed bool + err error +} + +func (m *mockRateLimitStore) Allow(_ context.Context, _ string, _ float64, _ int) (bool, error) { + return m.allowed, m.err +} + +func (m *mockRateLimitStore) Cleanup(_ context.Context, _ time.Duration) error { return nil } + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +func init() { gin.SetMode(gin.TestMode) } + +// newEngine builds a test engine that injects user_id and optional pre-cached +// permissions before the middleware under test runs. This avoids needing to +// mock auth.ResolvePermissions (a package-level function). +func newEngine(userID string, perms map[string]bool, mw gin.HandlerFunc) *gin.Engine { + e := gin.New() + e.Use(func(c *gin.Context) { + if userID != "" { + c.Set("user_id", userID) + } + if perms != nil { + c.Set(permCacheKey, perms) + } + c.Next() + }) + e.Use(mw) + e.GET("/test", func(c *gin.Context) { + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + return e +} + +func bodyMap(w *httptest.ResponseRecorder) map[string]interface{} { + var m map[string]interface{} + json.Unmarshal(w.Body.Bytes(), &m) + return m +} + +// --------------------------------------------------------------------------- +// RequirePermission tests +// --------------------------------------------------------------------------- + +func TestRequirePermission_Allowed(t *testing.T) { + s := store.Stores{} // not used — perms are pre-cached + perms := map[string]bool{"notes.read": true} + engine := newEngine("user-1", perms, RequirePermission("notes.read", s)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } + body := bodyMap(w) + if body["ok"] != true { + t.Errorf("expected ok=true, got %v", body) + } +} + +func TestRequirePermission_Denied(t *testing.T) { + s := store.Stores{} + perms := map[string]bool{"notes.read": true} // no "notes.write" + engine := newEngine("user-1", perms, RequirePermission("notes.write", s)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } + body := bodyMap(w) + errMsg, _ := body["error"].(string) + if errMsg != "permission required: notes.write" { + t.Errorf("unexpected error message: %q", errMsg) + } +} + +func TestRequirePermission_CachingAcrossChain(t *testing.T) { + // Two RequirePermission middlewares in the same chain should share the + // cached permission map — the second must NOT trigger a fresh resolve. + s := store.Stores{} + perms := map[string]bool{"a.read": true, "b.read": true} + + e := gin.New() + e.Use(func(c *gin.Context) { + c.Set("user_id", "user-1") + c.Set(permCacheKey, perms) + c.Next() + }) + e.Use(RequirePermission("a.read", s)) + e.Use(RequirePermission("b.read", s)) + e.GET("/test", func(c *gin.Context) { + // Verify the cached permissions are still present and correct. + resolved := GetResolvedPermissions(c) + if resolved == nil { + c.JSON(http.StatusInternalServerError, gin.H{"error": "cache missing"}) + return + } + if !resolved["a.read"] || !resolved["b.read"] { + c.JSON(http.StatusInternalServerError, gin.H{"error": "cache incomplete"}) + return + } + c.JSON(http.StatusOK, gin.H{"ok": true}) + }) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + e.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d (body: %s)", w.Code, w.Body.String()) + } +} + +// --------------------------------------------------------------------------- +// GetResolvedPermissions tests +// --------------------------------------------------------------------------- + +func TestGetResolvedPermissions_NilWhenNotSet(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + if got := GetResolvedPermissions(c); got != nil { + t.Errorf("expected nil, got %v", got) + } +} + +func TestGetResolvedPermissions_ReturnsCached(t *testing.T) { + w := httptest.NewRecorder() + c, _ := gin.CreateTestContext(w) + want := map[string]bool{"x": true} + c.Set(permCacheKey, want) + + got := GetResolvedPermissions(c) + if got == nil || !got["x"] { + t.Errorf("expected cached perms, got %v", got) + } +} + +// --------------------------------------------------------------------------- +// RequireAdmin tests +// --------------------------------------------------------------------------- + +func TestRequireAdmin_Allowed(t *testing.T) { + s := store.Stores{} + perms := map[string]bool{auth.PermSurfaceAdminAccess: true} + engine := newEngine("admin-1", perms, RequireAdmin(s)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestRequireAdmin_Denied(t *testing.T) { + s := store.Stores{} + perms := map[string]bool{"notes.read": true} // no admin perm + engine := newEngine("user-1", perms, RequireAdmin(s)) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusForbidden { + t.Errorf("expected 403, got %d", w.Code) + } + body := bodyMap(w) + errMsg, _ := body["error"].(string) + if errMsg != "admin access required" { + t.Errorf("unexpected error message: %q", errMsg) + } +} + +// --------------------------------------------------------------------------- +// RateLimiter.Limit tests +// --------------------------------------------------------------------------- + +func TestRateLimiter_Allowed(t *testing.T) { + rl := NewRateLimiter(&mockRateLimitStore{allowed: true}, 10, 20) + engine := newEngine("", nil, rl.Limit()) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200, got %d", w.Code) + } +} + +func TestRateLimiter_Exceeded(t *testing.T) { + rl := NewRateLimiter(&mockRateLimitStore{allowed: false}, 10, 20) + engine := newEngine("", nil, rl.Limit()) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusTooManyRequests { + t.Errorf("expected 429, got %d", w.Code) + } + if ra := w.Header().Get("Retry-After"); ra != "1" { + t.Errorf("expected Retry-After=1, got %q", ra) + } + body := bodyMap(w) + errMsg, _ := body["error"].(string) + if errMsg != "rate limit exceeded" { + t.Errorf("unexpected error message: %q", errMsg) + } +} + +func TestRateLimiter_StoreError_FailOpen(t *testing.T) { + rl := NewRateLimiter(&mockRateLimitStore{err: errors.New("db down")}, 10, 20) + engine := newEngine("", nil, rl.Limit()) + + w := httptest.NewRecorder() + req := httptest.NewRequest("GET", "/test", nil) + engine.ServeHTTP(w, req) + + if w.Code != http.StatusOK { + t.Errorf("expected 200 (fail-open), got %d", w.Code) + } +} diff --git a/server/middleware/prometheus.go b/server/middleware/prometheus.go index 21b3988..e4022f8 100644 --- a/server/middleware/prometheus.go +++ b/server/middleware/prometheus.go @@ -11,7 +11,7 @@ import ( // Prometheus returns a Gin middleware that records HTTP request metrics. // Uses c.FullPath() (Gin's registered route pattern, e.g. -// "/api/v1/channels/:id") to avoid label cardinality explosion from +// "/api/v1/workflows/:id") to avoid label cardinality explosion from // path parameters. func Prometheus() gin.HandlerFunc { return func(c *gin.Context) { diff --git a/server/pages/pages.go b/server/pages/pages.go index ed6e332..e0253cd 100644 --- a/server/pages/pages.go +++ b/server/pages/pages.go @@ -668,10 +668,10 @@ type WorkflowLandingPageData struct { // The middleware (AuthOrSession) has already created/resumed the session. func (e *Engine) RenderWorkflow() gin.HandlerFunc { return func(c *gin.Context) { - channelID := c.GetString("channel_id") + // :id is the public entry token (from /w/:id) + channelID := c.Param("id") sessionID := c.GetString("session_id") - // Load channel metadata var title, description string if title == "" { title = "Workflow" diff --git a/server/pages/templates/workflow.html b/server/pages/templates/workflow.html index 3eb948b..e2f89a7 100644 --- a/server/pages/templates/workflow.html +++ b/server/pages/templates/workflow.html @@ -474,7 +474,7 @@ } try { - var resp = await fetch(BASE + '/api/v1/w/' + CHAN_ID + '/form-submit', { + var resp = await fetch(BASE + '/api/v1/public/workflows/advance/' + CHAN_ID, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ data: data }), @@ -561,11 +561,8 @@ addMessage('user', text, '{{.Data.SessionName}}'); try { - var resp = await fetch(BASE + '/api/v1/w/' + CHAN_ID + '/completions', { - method: 'POST', - headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ channel_id: CHAN_ID, content: text, stream: false }), - }); + // TODO(v0.8+): form_chat completions endpoint not yet implemented + var resp = { ok: false, statusText: 'Chat mode not yet available', json: function() { return Promise.resolve({ error: 'Chat mode not yet available' }); } }; if (resp.ok) { var data = await resp.json(); if (data.content) { @@ -627,7 +624,7 @@ html += '

Collected Data

'; try { - var resp = await fetch(BASE + '/api/v1/channels/' + CHAN_ID + '/workflow/status', { + var resp = await fetch(BASE + '/api/v1/public/workflows/resume/' + CHAN_ID, { headers: { 'Content-Type': 'application/json' }, }); if (resp.ok) { @@ -684,7 +681,7 @@ document.getElementById('reviewAdvanceBtn').addEventListener('click', async function() { this.disabled = true; try { - var r = await fetch(BASE + '/api/v1/channels/' + CHAN_ID + '/workflow/advance', { + var r = await fetch(BASE + '/api/v1/public/workflows/advance/' + CHAN_ID, { method: 'POST', headers: { 'Content-Type': 'application/json' }, body: JSON.stringify({ data: {} }), }); @@ -705,9 +702,9 @@ if (!reason) return; this.disabled = true; try { - var r = await fetch(BASE + '/api/v1/channels/' + CHAN_ID + '/workflow/reject', { + var r = await fetch(BASE + '/api/v1/public/workflows/advance/' + CHAN_ID, { method: 'POST', headers: { 'Content-Type': 'application/json' }, - body: JSON.stringify({ reason: reason }), + body: JSON.stringify({ data: { _action: 'reject', reason: reason } }), }); if (r.ok) { actionPanel.innerHTML = '

Rejected

Sent back for revision.

'; diff --git a/server/sandbox/db_module.go b/server/sandbox/db_module.go index e8a3307..e55ab52 100644 --- a/server/sandbox/db_module.go +++ b/server/sandbox/db_module.go @@ -15,8 +15,8 @@ // rows = db.view("users", filters={"id": "abc"}) // // All table access is scoped to ext_{package_id}_{table_name}. -// Platform views (ext_view_users, ext_view_channels) are accessible -// via db.view() and are read-only regardless of permission level. +// Platform views (ext_view_users) are accessible via db.view() +// and are read-only regardless of permission level. // // Queries are parameterized — no raw SQL is ever accepted from scripts. // Dialect differences (placeholder style) are handled internally. @@ -44,8 +44,7 @@ type DBModuleConfig struct { // allowedViews is the set of platform views extensions may query via db.view(). var allowedViews = map[string]bool{ - "users": true, - "channels": true, + "users": true, } // BuildDBModule creates the "db" starlark module for an extension. @@ -435,7 +434,7 @@ func dbView(ctx context.Context, cfg DBModuleConfig) func(*starlark.Thread, *sta } if !allowedViews[viewName] { - return nil, fmt.Errorf("db.view: unknown view %q (allowed: users, channels)", viewName) + return nil, fmt.Errorf("db.view: unknown view %q (allowed: users)", viewName) } physView := "ext_view_" + viewName diff --git a/server/sandbox/runner.go b/server/sandbox/runner.go index a69bfee..b8925ff 100644 --- a/server/sandbox/runner.go +++ b/server/sandbox/runner.go @@ -61,9 +61,6 @@ type RunContext struct { // UserID is the acting user for provider resolution (BYOK chain). UserID string - // ChannelID is the context identifier, if any. - ChannelID string - // TeamID is the team context for settings cascade resolution. // When set, team-scoped package settings are included in the cascade. TeamID string diff --git a/server/sandbox/workflow_module.go b/server/sandbox/workflow_module.go index 6f1fa93..c7b2ef4 100644 --- a/server/sandbox/workflow_module.go +++ b/server/sandbox/workflow_module.go @@ -187,4 +187,4 @@ func goValToStarlark(v interface{}) starlark.Value { } // workflowRoute routes the workflow to a named stage. -// Starlark: workflow.route(channel_id, target_stage, reason) +// Starlark: workflow.route(instance_id, target_stage, reason) diff --git a/server/storage/storage.go b/server/storage/storage.go index 3f3e09f..59ccc43 100644 --- a/server/storage/storage.go +++ b/server/storage/storage.go @@ -24,7 +24,7 @@ var ( // // Keys are slash-delimited paths relative to the storage root: // -// files/{channel_id}/{file_id}_{filename} +// ext/{package_id}/{file_id}_{filename} // // Implementations must create intermediate directories as needed. type ObjectStore interface { @@ -43,8 +43,8 @@ type ObjectStore interface { Delete(ctx context.Context, key string) error // DeletePrefix removes all objects under the given prefix. - // Used for channel deletion (bulk cleanup). - // Example: DeletePrefix(ctx, "files/{channel_id}/") + // Used for bulk cleanup (e.g. extension uninstall). + // Example: DeletePrefix(ctx, "ext/{package_id}/") DeletePrefix(ctx context.Context, prefix string) error // Exists checks if an object exists at key without reading it. diff --git a/server/webhook/webhook.go b/server/webhook/webhook.go index 4a74b2b..2845fcf 100644 --- a/server/webhook/webhook.go +++ b/server/webhook/webhook.go @@ -29,7 +29,6 @@ type Payload struct { RunID string `json:"run_id,omitempty"` TaskName string `json:"task_name,omitempty"` WorkflowID string `json:"workflow_id,omitempty"` - ChannelID string `json:"channel_id,omitempty"` Status string `json:"status"` CompletedAt time.Time `json:"completed_at"` Output string `json:"output,omitempty"` // last assistant message or relay payload diff --git a/server/workflow/routing_test.go b/server/workflow/routing_test.go new file mode 100644 index 0000000..b6f3eb4 --- /dev/null +++ b/server/workflow/routing_test.go @@ -0,0 +1,497 @@ +package workflow + +import ( + "encoding/json" + "testing" + + "armature/models" +) + +// ── helpers ──────────────────────────────────────────── + +func mkStages(names ...string) []models.WorkflowStage { + out := make([]models.WorkflowStage, len(names)) + for i, n := range names { + out[i] = models.WorkflowStage{ + ID: n, + WorkflowID: "wf-test", + Ordinal: i, + Name: n, + } + } + return out +} + +func withBranchRules(stages []models.WorkflowStage, idx int, rules []Condition) []models.WorkflowStage { + b, _ := json.Marshal(rules) + stages[idx].BranchRules = b + return stages +} + +func mustJSON(v any) json.RawMessage { + b, _ := json.Marshal(v) + return b +} + +// ── ResolveNextStage ─────────────────────────────────── + +func TestResolveNextStage_NoRules(t *testing.T) { + stages := mkStages("intake", "review", "done") + got, err := ResolveNextStage(stages, 0, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("expected ordinal 1, got %d", got) + } +} + +func TestResolveNextStage_EmptyRulesArray(t *testing.T) { + stages := mkStages("intake", "review", "done") + stages[0].BranchRules = json.RawMessage(`[]`) + got, err := ResolveNextStage(stages, 0, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("expected ordinal 1, got %d", got) + } +} + +func TestResolveNextStage_SingleMatchingRule(t *testing.T) { + stages := mkStages("intake", "review", "escalation", "done") + rules := []Condition{{Field: "priority", Op: "eq", Value: "high", TargetStage: "escalation"}} + withBranchRules(stages, 0, rules) + + data := mustJSON(map[string]any{"priority": "high"}) + got, err := ResolveNextStage(stages, 0, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 2 { + t.Errorf("expected ordinal 2 (escalation), got %d", got) + } +} + +func TestResolveNextStage_FirstMatchWins(t *testing.T) { + stages := mkStages("intake", "fast-track", "escalation", "done") + rules := []Condition{ + {Field: "priority", Op: "eq", Value: "high", TargetStage: "fast-track"}, + {Field: "priority", Op: "eq", Value: "high", TargetStage: "escalation"}, + } + withBranchRules(stages, 0, rules) + + data := mustJSON(map[string]any{"priority": "high"}) + got, err := ResolveNextStage(stages, 0, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("expected ordinal 1 (fast-track), got %d", got) + } +} + +func TestResolveNextStage_NoMatchFallback(t *testing.T) { + stages := mkStages("intake", "review", "done") + rules := []Condition{{Field: "priority", Op: "eq", Value: "critical", TargetStage: "done"}} + withBranchRules(stages, 0, rules) + + data := mustJSON(map[string]any{"priority": "low"}) + got, err := ResolveNextStage(stages, 0, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("expected fallback ordinal 1, got %d", got) + } +} + +func TestResolveNextStage_OutOfBounds(t *testing.T) { + stages := mkStages("intake", "done") + + tests := []struct { + name string + current int + want int + }{ + {"negative", -1, 0}, + {"beyond end", 5, 6}, + {"equal to len", 2, 3}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ResolveNextStage(stages, tt.current, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("expected %d, got %d", tt.want, got) + } + }) + } +} + +func TestResolveNextStage_NilAndEmptyStageData(t *testing.T) { + stages := mkStages("intake", "review", "done") + rules := []Condition{{Field: "status", Op: "exists", TargetStage: "done"}} + withBranchRules(stages, 0, rules) + + // nil stageData — "exists" should be false, fallback to ordinal+1 + got, err := ResolveNextStage(stages, 0, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("nil data: expected 1, got %d", got) + } + + // empty object — "exists" should still be false + got, err = ResolveNextStage(stages, 0, json.RawMessage(`{}`)) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 1 { + t.Errorf("empty data: expected 1, got %d", got) + } +} + +func TestResolveNextStage_TerminalStage(t *testing.T) { + stages := mkStages("intake", "review", "done") + // At last stage with no rules, ordinal+1 == len(stages) which is past end + got, err := ResolveNextStage(stages, 2, nil) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 3 { + t.Errorf("expected terminal ordinal 3, got %d", got) + } +} + +func TestResolveNextStage_TargetByOrdinalString(t *testing.T) { + stages := mkStages("intake", "review", "done") + rules := []Condition{{Field: "skip", Op: "eq", Value: "yes", TargetStage: "2"}} + withBranchRules(stages, 0, rules) + + data := mustJSON(map[string]any{"skip": "yes"}) + got, err := ResolveNextStage(stages, 0, data) + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != 2 { + t.Errorf("expected ordinal 2, got %d", got) + } +} + +func TestResolveNextStage_InvalidTarget(t *testing.T) { + stages := mkStages("intake", "review", "done") + rules := []Condition{{Field: "go", Op: "eq", Value: "yes", TargetStage: "nonexistent"}} + withBranchRules(stages, 0, rules) + + data := mustJSON(map[string]any{"go": "yes"}) + _, err := ResolveNextStage(stages, 0, data) + if err == nil { + t.Fatalf("expected error for invalid target, got nil") + } +} + +// ── ResolveStageByName ───────────────────────────────── + +func TestResolveStageByName(t *testing.T) { + stages := mkStages("Intake", "Review", "Done") + + tests := []struct { + name string + input string + want int + wantErr bool + }{ + {"exact match", "Intake", 0, false}, + {"case insensitive lower", "review", 1, false}, + {"case insensitive upper", "DONE", 2, false}, + {"mixed case", "rEvIeW", 1, false}, + {"leading whitespace", " Intake", 0, false}, + {"trailing whitespace", "Done ", 2, false}, + {"both whitespace", " Review ", 1, false}, + {"not found", "nonexistent", 0, true}, + {"empty string", "", 0, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got, err := ResolveStageByName(stages, tt.input) + if tt.wantErr { + if err == nil { + t.Fatalf("expected error, got nil") + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + if got != tt.want { + t.Errorf("expected ordinal %d, got %d", tt.want, got) + } + }) + } +} + +// ── ParseStageConfig ─────────────────────────────────── + +func TestParseStageConfig(t *testing.T) { + tests := []struct { + name string + input json.RawMessage + checkField string // which field to validate + wantStr string + wantInt int + }{ + { + name: "valid with auto_assign", + input: json.RawMessage(`{"auto_assign":"team-lead","required_role":"manager"}`), + checkField: "auto_assign", + wantStr: "team-lead", + }, + { + name: "valid with required_role", + input: json.RawMessage(`{"required_role":"admin"}`), + checkField: "required_role", + wantStr: "admin", + }, + { + name: "valid with validation", + input: json.RawMessage(`{"validation":{"required_approvals":3}}`), + checkField: "validation_approvals", + wantInt: 3, + }, + { + name: "empty input", + input: nil, + checkField: "auto_assign", + wantStr: "", + }, + { + name: "null string", + input: json.RawMessage(`null`), + checkField: "auto_assign", + wantStr: "", + }, + { + name: "empty object", + input: json.RawMessage(`{}`), + checkField: "auto_assign", + wantStr: "", + }, + { + name: "invalid json", + input: json.RawMessage(`{bad json`), + checkField: "auto_assign", + wantStr: "", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + sc := ParseStageConfig(tt.input) + switch tt.checkField { + case "auto_assign": + if sc.AutoAssign != tt.wantStr { + t.Errorf("AutoAssign: expected %q, got %q", tt.wantStr, sc.AutoAssign) + } + case "required_role": + if sc.RequiredRole != tt.wantStr { + t.Errorf("RequiredRole: expected %q, got %q", tt.wantStr, sc.RequiredRole) + } + case "validation_approvals": + if sc.Validation == nil { + t.Fatalf("expected Validation to be set") + } + if sc.Validation.RequiredApprovals != tt.wantInt { + t.Errorf("RequiredApprovals: expected %d, got %d", tt.wantInt, sc.Validation.RequiredApprovals) + } + } + }) + } +} + +// ── evaluateCondition ────────────────────────────────── + +func TestEvaluateCondition_Exists(t *testing.T) { + data := map[string]any{"name": "alice", "age": 30} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"exists present", Condition{Field: "name", Op: "exists"}, true}, + {"exists absent", Condition{Field: "missing", Op: "exists"}, false}, + {"not_exists absent", Condition{Field: "missing", Op: "not_exists"}, true}, + {"not_exists present", Condition{Field: "name", Op: "not_exists"}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} + +func TestEvaluateCondition_EqNeq(t *testing.T) { + data := map[string]any{"status": "open", "count": float64(5)} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"eq string match", Condition{Field: "status", Op: "eq", Value: "open"}, true}, + {"eq string no match", Condition{Field: "status", Op: "eq", Value: "closed"}, false}, + {"neq string match", Condition{Field: "status", Op: "neq", Value: "closed"}, true}, + {"neq string no match", Condition{Field: "status", Op: "neq", Value: "open"}, false}, + {"eq numeric match", Condition{Field: "count", Op: "eq", Value: float64(5)}, true}, + {"eq numeric no match", Condition{Field: "count", Op: "eq", Value: float64(10)}, false}, + {"neq numeric", Condition{Field: "count", Op: "neq", Value: float64(3)}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} + +func TestEvaluateCondition_NumericComparisons(t *testing.T) { + data := map[string]any{"score": float64(75)} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"gt true", Condition{Field: "score", Op: "gt", Value: float64(50)}, true}, + {"gt false equal", Condition{Field: "score", Op: "gt", Value: float64(75)}, false}, + {"gt false less", Condition{Field: "score", Op: "gt", Value: float64(100)}, false}, + {"lt true", Condition{Field: "score", Op: "lt", Value: float64(100)}, true}, + {"lt false equal", Condition{Field: "score", Op: "lt", Value: float64(75)}, false}, + {"lt false greater", Condition{Field: "score", Op: "lt", Value: float64(50)}, false}, + {"gte true greater", Condition{Field: "score", Op: "gte", Value: float64(50)}, true}, + {"gte true equal", Condition{Field: "score", Op: "gte", Value: float64(75)}, true}, + {"gte false", Condition{Field: "score", Op: "gte", Value: float64(100)}, false}, + {"lte true less", Condition{Field: "score", Op: "lte", Value: float64(100)}, true}, + {"lte true equal", Condition{Field: "score", Op: "lte", Value: float64(75)}, true}, + {"lte false", Condition{Field: "score", Op: "lte", Value: float64(50)}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} + +func TestEvaluateCondition_In(t *testing.T) { + data := map[string]any{"role": "admin"} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"in list match", Condition{Field: "role", Op: "in", Value: []any{"admin", "manager"}}, true}, + {"in list no match", Condition{Field: "role", Op: "in", Value: []any{"user", "guest"}}, false}, + {"in empty list", Condition{Field: "role", Op: "in", Value: []any{}}, false}, + {"in non-list value", Condition{Field: "role", Op: "in", Value: "admin"}, false}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} + +func TestEvaluateCondition_Contains(t *testing.T) { + data := map[string]any{"description": "urgent: fix production bug"} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"contains match", Condition{Field: "description", Op: "contains", Value: "urgent"}, true}, + {"contains substring", Condition{Field: "description", Op: "contains", Value: "production"}, true}, + {"contains no match", Condition{Field: "description", Op: "contains", Value: "minor"}, false}, + {"contains empty target", Condition{Field: "description", Op: "contains", Value: ""}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} + +func TestEvaluateCondition_MissingField(t *testing.T) { + data := map[string]any{"present": "yes"} + + ops := []string{"eq", "neq", "gt", "lt", "gte", "lte", "in", "contains"} + for _, op := range ops { + t.Run(op, func(t *testing.T) { + cond := Condition{Field: "absent", Op: op, Value: "something"} + got := evaluateCondition(cond, data) + if got != false { + t.Errorf("op %q on missing field: expected false, got true", op) + } + }) + } +} + +func TestEvaluateCondition_UnknownOperator(t *testing.T) { + data := map[string]any{"field": "value"} + cond := Condition{Field: "field", Op: "regex", Value: ".*"} + got := evaluateCondition(cond, data) + if got != false { + t.Errorf("unknown operator: expected false, got true") + } +} + +func TestEvaluateCondition_EmptyData(t *testing.T) { + data := map[string]any{} + cond := Condition{Field: "anything", Op: "eq", Value: "test"} + got := evaluateCondition(cond, data) + if got != false { + t.Errorf("empty data: expected false, got true") + } +} + +func TestEvaluateCondition_NumericStringComparison(t *testing.T) { + // JSON unmarshalling produces float64 for numbers; test string numeric values too + data := map[string]any{"amount": "100.5"} + + tests := []struct { + name string + cond Condition + want bool + }{ + {"gt string num", Condition{Field: "amount", Op: "gt", Value: float64(50)}, true}, + {"lt string num", Condition{Field: "amount", Op: "lt", Value: float64(200)}, true}, + {"eq string num", Condition{Field: "amount", Op: "eq", Value: "100.5"}, true}, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + got := evaluateCondition(tt.cond, data) + if got != tt.want { + t.Errorf("expected %v, got %v", tt.want, got) + } + }) + } +} diff --git a/src/js/sw/surfaces/admin/backup.js b/src/js/sw/surfaces/admin/backup.js index d572e29..ce9e219 100644 --- a/src/js/sw/surfaces/admin/backup.js +++ b/src/js/sw/surfaces/admin/backup.js @@ -30,12 +30,12 @@ export default function BackupSection() { sw.toast('Backup created', 'success'); load(); } else { - // Stream download + // Stream download — use _getToken() for raw bearer token const url = '/api/v1/admin/backup'; - const token = sw.auth.token(); + const token = sw.auth._getToken(); const resp = await fetch(url, { method: 'POST', - headers: { 'Authorization': 'Bearer ' + token }, + headers: token ? { 'Authorization': 'Bearer ' + token } : {}, }); if (!resp.ok) throw new Error('Backup failed'); const blob = await resp.blob(); @@ -54,9 +54,9 @@ export default function BackupSection() { } async function downloadBackup(name) { - const token = sw.auth.token(); + const token = sw.auth._getToken(); const resp = await fetch(sw.api.admin.backup.download(name), { - headers: { 'Authorization': 'Bearer ' + token }, + headers: token ? { 'Authorization': 'Bearer ' + token } : {}, }); if (!resp.ok) { sw.toast('Download failed', 'error'); return; } const blob = await resp.blob(); diff --git a/src/js/sw/surfaces/admin/index.js b/src/js/sw/surfaces/admin/index.js index eaa0889..f65157a 100644 --- a/src/js/sw/surfaces/admin/index.js +++ b/src/js/sw/surfaces/admin/index.js @@ -20,14 +20,14 @@ import { DialogStack } from '../../shell/dialog-stack.js'; const ADMIN_SECTIONS = { people: ['users', 'teams', 'groups'], workflows: ['workflows'], - system: ['settings', 'storage', 'packages', 'connections', 'broadcast', 'backup'], + system: ['settings', 'packages', 'connections', 'broadcast', 'backup'], monitoring: ['health', 'audit'], }; const ADMIN_LABELS = { users: 'Users', teams: 'Teams', groups: 'Groups', workflows: 'Workflows', - settings: 'Settings', storage: 'Storage', packages: 'Packages', + settings: 'Settings', packages: 'Packages', connections: 'Connections', broadcast: 'Broadcast', backup: 'Backup', health: 'Health', audit: 'Audit', @@ -63,7 +63,6 @@ const sectionModules = { groups: () => import(`./groups.js${_v}`), workflows: () => import(`./workflows.js${_v}`), settings: () => import(`./settings.js${_v}`), - storage: () => import(`./storage.js${_v}`), packages: () => import(`./packages.js${_v}`), connections: () => import(`./connections.js${_v}`), broadcast: () => import(`./broadcast.js${_v}`), diff --git a/src/js/sw/surfaces/admin/storage.js b/src/js/sw/surfaces/admin/storage.js deleted file mode 100644 index 6d62e25..0000000 --- a/src/js/sw/surfaces/admin/storage.js +++ /dev/null @@ -1,76 +0,0 @@ -/** - * Admin > System > Storage - */ -const { html } = window; -const { useState, useEffect, useCallback } = hooks; - -export default function StorageSection() { - const [status, setStatus] = useState(null); - const [orphans, setOrphans] = useState(null); - const [extraction, setExtraction] = useState(null); - const [loading, setLoading] = useState(true); - - const load = useCallback(async () => { - try { - const [s, o, e] = await Promise.all([ - sw.api.admin.storage.status(), - sw.api.admin.storage.orphans(), - sw.api.admin.storage.extraction().catch(() => null), - ]); - setStatus(s); - setOrphans(o); - setExtraction(e); - } catch (e) { sw.toast(e.message, 'error'); } - finally { setLoading(false); } - }, []); - - useEffect(() => { load(); }, []); - - async function cleanup() { - const ok = await sw.confirm('Remove orphaned blobs?'); - if (!ok) return; - try { - await sw.api.admin.storage.cleanup(); - sw.toast('Cleanup complete', 'success'); - load(); - } catch (e) { sw.toast(e.message, 'error'); } - } - - function formatBytes(b) { - if (!b) return '0 B'; - if (b < 1024) return b + ' B'; - if (b < 1048576) return (b / 1024).toFixed(1) + ' KB'; - if (b < 1073741824) return (b / 1048576).toFixed(1) + ' MB'; - return (b / 1073741824).toFixed(2) + ' GB'; - } - - if (loading) return html`
Loading\u2026
`; - - return html` -
- ${status && html` -
-
${status.backend || '\u2014'}
Backend
-
${status.healthy ? 'Healthy' : 'Unhealthy'}
Status
-
${status.file_count ?? '\u2014'}
Files
-
${formatBytes(status.total_bytes)}
Total Size
-
- `} - - ${orphans && html` -
-

Orphaned Blobs

-

${orphans.count || 0} orphans found

- ${(orphans.count || 0) > 0 && html``} -
- `} - - ${extraction && html` -
-

Extraction Queue

- 
${JSON.stringify(extraction, null, 2)}
-
- `} -
- `; -}