From f3a7d43ebcd098da09a67a28193562df900a3e7b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Salavisa?= Date: Wed, 12 Aug 2026 22:27:44 +0100 Subject: [PATCH] test(office): cover config import, sync, and HTTP surface (#2582) Co-authored-by: Kandev Agent Co-authored-by: Claude Opus 5 (1M context) --- .../internal/office/config/handler_test.go | 360 +++++++++++ .../internal/office/config/helpers_test.go | 438 +++++++++++++ .../internal/office/config/import_test.go | 453 ++++++++++++++ .../internal/office/config/service_test.go | 411 +++++++++++++ .../office/config/sync_helpers_test.go | 257 ++++++++ .../internal/office/config/sync_test.go | 573 ++++++++++++++++++ .../internal/office/config/sync_util_test.go | 325 ++++++++++ .../office/config/write_errors_test.go | 172 ++++++ 8 files changed, 2989 insertions(+) create mode 100644 apps/backend/internal/office/config/handler_test.go create mode 100644 apps/backend/internal/office/config/helpers_test.go create mode 100644 apps/backend/internal/office/config/import_test.go create mode 100644 apps/backend/internal/office/config/service_test.go create mode 100644 apps/backend/internal/office/config/sync_helpers_test.go create mode 100644 apps/backend/internal/office/config/sync_test.go create mode 100644 apps/backend/internal/office/config/sync_util_test.go create mode 100644 apps/backend/internal/office/config/write_errors_test.go diff --git a/apps/backend/internal/office/config/handler_test.go b/apps/backend/internal/office/config/handler_test.go new file mode 100644 index 0000000000..978cb12eed --- /dev/null +++ b/apps/backend/internal/office/config/handler_test.go @@ -0,0 +1,360 @@ +package config + +import ( + "context" + "encoding/json" + "net/http" + "net/http/httptest" + "strings" + "testing" + + "github.com/gin-gonic/gin" + + "github.com/kandev/kandev/internal/common/logger" +) + +// newTestRouter mounts the config routes over a fresh service. +func newTestRouter(t *testing.T) (*gin.Engine, *testEnv) { + t.Helper() + gin.SetMode(gin.TestMode) + env := newTestEnv(t) + router := gin.New() + RegisterRoutes(router.Group("/api/v1"), NewHandler(env.svc, logger.Default())) + return router, env +} + +// do issues a request and returns the recorder. +func do(t *testing.T, router *gin.Engine, method, path, body string) *httptest.ResponseRecorder { + t.Helper() + req := httptest.NewRequest(method, path, strings.NewReader(body)) + req.Header.Set("Content-Type", "application/json") + rec := httptest.NewRecorder() + router.ServeHTTP(rec, req) + return rec +} + +// decodeBody unmarshals a JSON response body, failing on malformed output. +func decodeBody(t *testing.T, rec *httptest.ResponseRecorder, out interface{}) { + t.Helper() + if err := json.Unmarshal(rec.Body.Bytes(), out); err != nil { + t.Fatalf("decode body %q: %v", rec.Body.String(), err) + } +} + +func assertStatus(t *testing.T, rec *httptest.ResponseRecorder, want int) { + t.Helper() + if rec.Code != want { + t.Fatalf("status: got %d, want %d (body: %s)", rec.Code, want, rec.Body.String()) + } +} + +// assertErrorBody checks a failure response carries a non-empty error field. +func assertErrorBody(t *testing.T, rec *httptest.ResponseRecorder) { + t.Helper() + var body struct { + Error string `json:"error"` + } + decodeBody(t, rec, &body) + if body.Error == "" { + t.Errorf("expected a non-empty error field, got %s", rec.Body.String()) + } +} + +func TestNewHandler_WiresDependencies(t *testing.T) { + env := newTestEnv(t) + h := NewHandler(env.svc, logger.Default()) + if h.svc != env.svc { + t.Error("service not wired") + } + if h.logger == nil { + t.Error("logger not wired") + } +} + +func TestRegisterRoutes_RegistersEveryEndpoint(t *testing.T) { + router, _ := newTestRouter(t) + + want := map[string]string{ + "GET /api/v1/workspaces/:wsId/config/export": "", + "GET /api/v1/workspaces/:wsId/config/export/zip": "", + "POST /api/v1/workspaces/:wsId/config/preview": "", + "POST /api/v1/workspaces/:wsId/config/import": "", + "GET /api/v1/workspaces/:wsId/config/sync/incoming": "", + "GET /api/v1/workspaces/:wsId/config/sync/outgoing": "", + "POST /api/v1/workspaces/:wsId/config/sync/import-fs": "", + "POST /api/v1/workspaces/:wsId/config/sync/export-fs": "", + } + for _, r := range router.Routes() { + delete(want, r.Method+" "+r.Path) + } + for missing := range want { + t.Errorf("route not registered: %s", missing) + } +} + +func TestHandler_ExportConfig(t *testing.T) { + router, env := newTestRouter(t) + seedFullDBState(t, env, testWorkspaceID) + + rec := do(t, router, http.MethodGet, "/api/v1/workspaces/"+testWorkspaceID+"/config/export", "") + assertStatus(t, rec, http.StatusOK) + + var body struct { + Bundle ConfigBundle `json:"bundle"` + } + decodeBody(t, rec, &body) + // The path parameter reaches the service: it labels the settings block. + assertEqual(t, "settings name", body.Bundle.Settings.Name, testWorkspaceID) + assertEqual(t, "agents", len(body.Bundle.Agents), 2) + assertEqual(t, "skills", len(body.Bundle.Skills), 1) + assertEqual(t, "routines", len(body.Bundle.Routines), 1) + assertEqual(t, "projects", len(body.Bundle.Projects), 1) +} + +func TestHandler_ExportConfig_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodGet, "/api/v1/workspaces/ws/config/export", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_ExportConfigZip(t *testing.T) { + router, env := newTestRouter(t) + seedFullDBState(t, env, testWorkspaceID) + + rec := do(t, router, http.MethodGet, + "/api/v1/workspaces/"+testWorkspaceID+"/config/export/zip", "") + assertStatus(t, rec, http.StatusOK) + assertEqual(t, "content type", rec.Header().Get("Content-Type"), "application/zip") + assertEqual(t, "content disposition", rec.Header().Get("Content-Disposition"), + "attachment; filename=kandev-config.zip") + + entries := readZip(t, rec.Body) + if _, ok := entries[".kandev/agents/ada.yml"]; !ok { + t.Errorf("agent entry missing from the archive, got %v", entries) + } +} + +func TestHandler_ExportConfigZip_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodGet, "/api/v1/workspaces/ws/config/export/zip", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) + if got := rec.Header().Get("Content-Type"); strings.HasPrefix(got, "application/zip") { + t.Errorf("error response should not claim to be a zip, got %q", got) + } +} + +func TestHandler_PreviewImport(t *testing.T) { + router, env := newTestRouter(t) + seedAgent(t, env, testWorkspaceID, "ada") + + body := `{"agents":[{"name":"ada"},{"name":"grace"}],"skills":[{"slug":"triage"}]}` + rec := do(t, router, http.MethodPost, + "/api/v1/workspaces/"+testWorkspaceID+"/config/preview", body) + assertStatus(t, rec, http.StatusOK) + + var out struct { + Preview ImportPreview `json:"preview"` + } + decodeBody(t, rec, &out) + assertStrings(t, "agents updated", out.Preview.Agents.Updated, []string{"ada"}) + assertStrings(t, "agents created", out.Preview.Agents.Created, []string{"grace"}) + assertStrings(t, "skills created", out.Preview.Skills.Created, []string{"triage"}) + + // Preview must not have written anything. + agents, err := env.repo.ListAgentInstances(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "row count unchanged", len(agents), 1) +} + +func TestHandler_PreviewImport_MalformedBody(t *testing.T) { + router, _ := newTestRouter(t) + + for _, body := range []string{"{not json", "", `{"agents":"not-an-array"}`} { + rec := do(t, router, http.MethodPost, "/api/v1/workspaces/ws/config/preview", body) + assertStatus(t, rec, http.StatusBadRequest) + assertErrorBody(t, rec) + } +} + +func TestHandler_PreviewImport_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodPost, "/api/v1/workspaces/ws/config/preview", `{}`) + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_ApplyImport(t *testing.T) { + router, env := newTestRouter(t) + + body := `{"agents":[{"name":"ada","role":"cto","budget_monthly_cents":4200, + "max_concurrent_sessions":3,"executor_preference":"local_docker"}], + "projects":[{"name":"apollo","budget_cents":99000}]}` + rec := do(t, router, http.MethodPost, + "/api/v1/workspaces/"+testWorkspaceID+"/config/import", body) + assertStatus(t, rec, http.StatusOK) + + var out struct { + Result ImportResult `json:"result"` + } + decodeBody(t, rec, &out) + assertEqual(t, "created count", out.Result.CreatedCount, 2) + assertEqual(t, "updated count", out.Result.UpdatedCount, 0) + + // The workspace path parameter scopes the write. + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "agent role", string(agent.Role), "cto") + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", agent.MaxConcurrentSessions, 3) + assertEqual(t, "agent executor preference", agent.ExecutorPreference, "local_docker") + assertEqual(t, "project budget", + projectByName(t, env, testWorkspaceID, "apollo").BudgetCents, 99000) +} + +func TestHandler_ApplyImport_MalformedBody(t *testing.T) { + router, env := newTestRouter(t) + + rec := do(t, router, http.MethodPost, + "/api/v1/workspaces/"+testWorkspaceID+"/config/import", "{not json") + assertStatus(t, rec, http.StatusBadRequest) + assertErrorBody(t, rec) + + // Scoped to the workspace the request actually targeted: listing a + // workspace nothing writes to would pass no matter what the handler did. + agents, err := env.repo.ListAgentInstances(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "a rejected body writes nothing", len(agents), 0) +} + +func TestHandler_ApplyImport_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodPost, "/api/v1/workspaces/ws/config/import", + `{"agents":[{"name":"ada"}]}`) + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_SyncIncomingDiff(t *testing.T) { + router, env := newTestRouter(t) + seedFullFSState(t, env) + seedAgent(t, env, testWorkspaceID, "gone-agent") + + rec := do(t, router, http.MethodGet, + "/api/v1/workspaces/"+testWorkspaceID+"/config/sync/incoming", "") + assertStatus(t, rec, http.StatusOK) + + var out struct { + Diff SyncDiff `json:"diff"` + } + decodeBody(t, rec, &out) + assertEqual(t, "direction", out.Diff.Direction, "incoming") + assertStrings(t, "agents created", out.Diff.Preview.Agents.Created, []string{"ada"}) + assertStrings(t, "agents deleted", out.Diff.Preview.Agents.Deleted, []string{"gone-agent"}) + assertEqual(t, "errors omitted when empty", len(out.Diff.Errors), 0) +} + +func TestHandler_SyncIncomingDiff_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodGet, "/api/v1/workspaces/ws/config/sync/incoming", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_SyncOutgoingDiff(t *testing.T) { + router, env := newTestRouter(t) + seedAgent(t, env, testWorkspaceID, "ada") + + rec := do(t, router, http.MethodGet, + "/api/v1/workspaces/"+testWorkspaceID+"/config/sync/outgoing", "") + assertStatus(t, rec, http.StatusOK) + + var out struct { + Diff SyncDiff `json:"diff"` + } + decodeBody(t, rec, &out) + assertEqual(t, "direction", out.Diff.Direction, "outgoing") + assertStrings(t, "agents created", out.Diff.Preview.Agents.Created, []string{"ada"}) +} + +func TestHandler_SyncOutgoingDiff_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodGet, "/api/v1/workspaces/ws/config/sync/outgoing", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_SyncApplyIncoming(t *testing.T) { + router, env := newTestRouter(t) + seedFullFSState(t, env) + seedAgent(t, env, testWorkspaceID, "gone-agent") + + rec := do(t, router, http.MethodPost, + "/api/v1/workspaces/"+testWorkspaceID+"/config/sync/import-fs", "") + assertStatus(t, rec, http.StatusOK) + + var out struct { + Result ImportResult `json:"result"` + } + decodeBody(t, rec, &out) + assertEqual(t, "created count", out.Result.CreatedCount, 4) + + assertRemainingNames(t, env, testWorkspaceID, []string{"ada"}, []string{"code-review"}, + []string{"standup"}, []string{"apollo"}) +} + +func TestHandler_SyncApplyIncoming_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodPost, "/api/v1/workspaces/ws/config/sync/import-fs", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} + +func TestHandler_SyncApplyOutgoing(t *testing.T) { + router, env := newTestRouter(t) + seedFullDBState(t, env, testWorkspaceID) + + rec := do(t, router, http.MethodPost, + "/api/v1/workspaces/"+testWorkspaceID+"/config/sync/export-fs", "") + assertStatus(t, rec, http.StatusOK) + + var out struct { + Status string `json:"status"` + } + decodeBody(t, rec, &out) + assertEqual(t, "status", out.Status, "ok") + + bundle, _, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + assertStrings(t, "agents on disk", + sortedCopy(agentBundleNames(bundle.Agents)), []string{"ada", "grace"}) +} + +func TestHandler_SyncApplyOutgoing_ServiceError(t *testing.T) { + router, env := newTestRouter(t) + env.closeDB(t) + + rec := do(t, router, http.MethodPost, "/api/v1/workspaces/ws/config/sync/export-fs", "") + assertStatus(t, rec, http.StatusInternalServerError) + assertErrorBody(t, rec) +} diff --git a/apps/backend/internal/office/config/helpers_test.go b/apps/backend/internal/office/config/helpers_test.go new file mode 100644 index 0000000000..3da20cb895 --- /dev/null +++ b/apps/backend/internal/office/config/helpers_test.go @@ -0,0 +1,438 @@ +package config + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/jmoiron/sqlx" + _ "github.com/mattn/go-sqlite3" + + settingsstore "github.com/kandev/kandev/internal/agent/settings/store" + "github.com/kandev/kandev/internal/common/logger" + "github.com/kandev/kandev/internal/office/configloader" + "github.com/kandev/kandev/internal/office/models" + "github.com/kandev/kandev/internal/office/repository/sqlite" +) + +// testWorkspaceID is the DB workspace every fixture is written under. It is +// deliberately different from defaultWorkspaceName ("default"), which is the +// *filesystem* workspace directory the ConfigLoader reads — the two are +// separate namespaces and several tests depend on telling them apart. +const testWorkspaceID = "ws-test" + +// activityEntry records one shared.ActivityLogger.LogActivity call. +type activityEntry struct { + wsID string + actorType string + actorID string + action string + targetType string + targetID string + details string +} + +// recordingActivityLogger captures activity calls made during an import. +type recordingActivityLogger struct { + entries []activityEntry +} + +func (r *recordingActivityLogger) LogActivity( + _ context.Context, wsID, actorType, actorID, action, targetType, targetID, details string, +) { + r.entries = append(r.entries, activityEntry{ + wsID: wsID, actorType: actorType, actorID: actorID, + action: action, targetType: targetType, targetID: targetID, details: details, + }) +} + +func (r *recordingActivityLogger) LogActivityWithRun( + ctx context.Context, wsID, actorType, actorID, action, targetType, targetID, details, _, _ string, +) { + r.LogActivity(ctx, wsID, actorType, actorID, action, targetType, targetID, details) +} + +// testEnv bundles a ConfigService with everything a test needs to reach +// behind it: the DB it writes to, the on-disk config root the loader and +// writer share, and the activity log it emits to. +type testEnv struct { + svc *ConfigService + repo *sqlite.Repository + // db is the handle repository reads go through; wdb is the handle writes + // go through. newTestEnv points both at one in-memory database; + // newSplitTestEnv gives them separate handles so writes can be broken + // while reads keep working. + db *sqlx.DB + wdb *sqlx.DB + base string + loader *configloader.ConfigLoader + writer *configloader.FileWriter + activity *recordingActivityLogger +} + +// newTestEnv builds a ConfigService over an in-memory SQLite repo and a +// temp-dir config root containing an empty "default" FS workspace. +// +// Office agent CRUD reads/writes the unified agent_profiles table (ADR 0005 +// Wave C), so the settings-store schema is brought up on the same handles +// before the office repo initialises. +func newTestEnv(t *testing.T) *testEnv { + t.Helper() + db := newTestDB(t) + + repo, err := sqlite.NewWithDB(db, db, nil) + if err != nil { + t.Fatalf("new repo: %v", err) + } + + return newEnvAround(t, repo, db, db) +} + +// newSplitTestEnv builds the same service over a file-backed database opened +// twice: once for reads and once for writes. Closing the write handle with +// breakWrites leaves every List working while every Create/Update/Delete +// fails, which is the only way to reach the write-error branches of the +// import and prune paths. +func newSplitTestEnv(t *testing.T) *testEnv { + t.Helper() + dsn := filepath.Join(t.TempDir(), "office.db") + + writerDB := openTestDB(t, dsn) + if _, _, err := settingsstore.Provide(writerDB, writerDB, nil); err != nil { + t.Fatalf("settings store init: %v", err) + } + readerDB := openTestDB(t, dsn) + + repo, err := sqlite.NewWithDB(writerDB, readerDB, nil) + if err != nil { + t.Fatalf("new repo: %v", err) + } + return newEnvAround(t, repo, readerDB, writerDB) +} + +func openTestDB(t *testing.T, dsn string) *sqlx.DB { + t.Helper() + db, err := sqlx.Open("sqlite3", dsn) + if err != nil { + t.Fatalf("open sqlite %s: %v", dsn, err) + } + db.SetMaxOpenConns(1) + t.Cleanup(func() { _ = db.Close() }) + return db +} + +// newEnvAround wires a ConfigService around an already-built repository plus a +// fresh temp-dir config root holding an empty "default" FS workspace. +func newEnvAround(t *testing.T, repo *sqlite.Repository, readDB, writeDB *sqlx.DB) *testEnv { + t.Helper() + base := t.TempDir() + wsDir := filepath.Join(base, "workspaces", defaultWorkspaceName) + if err := os.MkdirAll(wsDir, 0o755); err != nil { + t.Fatalf("mkdir fs workspace: %v", err) + } + writeFile(t, filepath.Join(wsDir, "kandev.yml"), "name: default\n") + + loader := configloader.NewConfigLoader(base) + if err := loader.Load(); err != nil { + t.Fatalf("load config: %v", err) + } + writer := configloader.NewFileWriter(base, loader) + activity := &recordingActivityLogger{} + + return &testEnv{ + svc: NewConfigService(repo, loader, writer, logger.Default(), activity), + repo: repo, + db: readDB, + wdb: writeDB, + base: base, + loader: loader, + writer: writer, + activity: activity, + } +} + +// breakWrites closes the write handle of a split environment. +func (e *testEnv) breakWrites(t *testing.T) { + t.Helper() + if e.wdb == e.db { + t.Fatal("breakWrites requires newSplitTestEnv") + } + if err := e.wdb.Close(); err != nil { + t.Fatalf("close write handle: %v", err) + } +} + +// newTestDB opens an in-memory SQLite database with the settings-store +// schema installed. +func newTestDB(t *testing.T) *sqlx.DB { + t.Helper() + db, err := sqlx.Open("sqlite3", ":memory:") + if err != nil { + t.Fatalf("open sqlite: %v", err) + } + db.SetMaxOpenConns(1) + t.Cleanup(func() { _ = db.Close() }) + + if _, _, err := settingsstore.Provide(db, db, nil); err != nil { + t.Fatalf("settings store init: %v", err) + } + return db +} + +// closeDB closes the underlying database so every repository call fails. +// Used to exercise error propagation without a mock repository (the service +// holds a concrete *sqlite.Repository). +func (e *testEnv) closeDB(t *testing.T) { + t.Helper() + if err := e.db.Close(); err != nil { + t.Fatalf("close db: %v", err) + } +} + +// dropTable removes one table so calls touching that entity fail while the +// rest of the repository keeps working. This isolates a single stage of a +// multi-stage import, which closing the whole database cannot do. +func (e *testEnv) dropTable(t *testing.T, table string) { + t.Helper() + if _, err := e.db.Exec("DROP TABLE " + table); err != nil { + t.Fatalf("drop table %s: %v", table, err) + } +} + +// wsPath returns a path inside the on-disk "default" workspace. +func (e *testEnv) wsPath(parts ...string) string { + return filepath.Join(append([]string{e.base, "workspaces", defaultWorkspaceName}, parts...)...) +} + +// writeFSAgent writes an agent YAML file into the on-disk workspace. +func (e *testEnv) writeFSAgent(t *testing.T, name, body string) { + t.Helper() + writeFileIn(t, e.wsPath("agents"), name+".yml", body) +} + +// writeFSSkill writes a skills//SKILL.md file into the on-disk workspace. +func (e *testEnv) writeFSSkill(t *testing.T, slug, content string) { + t.Helper() + writeFileIn(t, e.wsPath("skills", slug), "SKILL.md", content) +} + +// writeFSRoutine writes a routine YAML file into the on-disk workspace. +func (e *testEnv) writeFSRoutine(t *testing.T, name, body string) { + t.Helper() + writeFileIn(t, e.wsPath("routines"), name+".yml", body) +} + +// writeFSProject writes a project YAML file into the on-disk workspace. +func (e *testEnv) writeFSProject(t *testing.T, name, body string) { + t.Helper() + writeFileIn(t, e.wsPath("projects"), name+".yml", body) +} + +func writeFileIn(t *testing.T, dir, name, body string) { + t.Helper() + if err := os.MkdirAll(dir, 0o755); err != nil { + t.Fatalf("mkdir %s: %v", dir, err) + } + writeFile(t, filepath.Join(dir, name), body) +} + +func writeFile(t *testing.T, path, body string) { + t.Helper() + if err := os.WriteFile(path, []byte(body), 0o644); err != nil { + t.Fatalf("write %s: %v", path, err) + } +} + +// fullBundle returns a ConfigBundle with every field of every entity set to a +// distinct non-zero value. Tests that import or export it assert field-by-field, +// so a field dropped anywhere in the pipeline fails loudly instead of silently +// arriving empty. +func fullBundle() *ConfigBundle { + return &ConfigBundle{ + Settings: SettingsConfig{Name: "Acme Workspace", Description: "the whole office"}, + Agents: []AgentConfig{{ + Name: "ada", + Role: "cto", + Icon: "🤖", + ReportsTo: "grace", + BudgetMonthlyCents: 4200, + MaxConcurrentSessions: 3, + DesiredSkills: `["go","sql"]`, + ExecutorPreference: "local_docker", + }}, + Skills: []SkillConfig{{ + Name: "Code Review", + Slug: "code-review", + Description: "reviews diffs", + SourceType: "inline", + Content: "# Code Review\n\nread the diff\n", + }}, + Routines: []RoutineConfig{{ + Name: "standup", + Description: "daily standup", + TaskTemplate: "summarise yesterday", + AssigneeName: "ada", + ConcurrencyPolicy: "skip", + }}, + Projects: []ProjectConfig{{ + Name: "apollo", + Description: "moon shot", + Status: "paused", + Color: "#ff00ff", + BudgetCents: 99000, + Repositories: `["kdlbs/kandev"]`, + ExecutorConfig: `{"type":"local_docker"}`, + LeadAgentName: "ada", + }}, + } +} + +// seedAgent inserts an office agent row and returns it. +func seedAgent(t *testing.T, e *testEnv, wsID, name string) *models.AgentInstance { + t.Helper() + agent := &models.AgentInstance{ + WorkspaceID: wsID, + Name: name, + Role: models.AgentRole("engineer"), + Icon: "🛠", + Status: models.AgentStatusIdle, + BudgetMonthlyCents: 100, + MaxConcurrentSessions: 1, + } + if err := e.repo.CreateAgentInstance(context.Background(), agent); err != nil { + t.Fatalf("seed agent %s: %v", name, err) + } + return agent +} + +// seedSkill inserts an office skill row and returns it. +func seedSkill(t *testing.T, e *testEnv, wsID, slug string) *models.Skill { + t.Helper() + skill := &models.Skill{ + WorkspaceID: wsID, + Name: slug, + Slug: slug, + SourceType: models.SkillSourceType("inline"), + Content: "seeded", + } + if err := e.repo.CreateSkill(context.Background(), skill); err != nil { + t.Fatalf("seed skill %s: %v", slug, err) + } + return skill +} + +// seedRoutine inserts an office routine row and returns it. +func seedRoutine(t *testing.T, e *testEnv, wsID, name string) *models.Routine { + t.Helper() + routine := &models.Routine{ + WorkspaceID: wsID, + Name: name, + Status: "active", + } + if err := e.repo.CreateRoutine(context.Background(), routine); err != nil { + t.Fatalf("seed routine %s: %v", name, err) + } + return routine +} + +// seedProject inserts an office project row and returns it. +func seedProject(t *testing.T, e *testEnv, wsID, name string) *models.Project { + t.Helper() + project := &models.Project{ + WorkspaceID: wsID, + Name: name, + Status: models.ProjectStatusActive, + } + if err := e.repo.CreateProject(context.Background(), project); err != nil { + t.Fatalf("seed project %s: %v", name, err) + } + return project +} + +// agentByName finds a seeded/imported agent by name, failing if absent. +func agentByName(t *testing.T, e *testEnv, wsID, name string) *models.AgentInstance { + t.Helper() + rows, err := e.repo.ListAgentInstances(context.Background(), wsID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + for _, a := range rows { + if a.Name == name { + return a + } + } + t.Fatalf("agent %q not found in workspace %q", name, wsID) + return nil +} + +// skillBySlug finds a seeded/imported skill by slug, failing if absent. +func skillBySlug(t *testing.T, e *testEnv, wsID, slug string) *models.Skill { + t.Helper() + rows, err := e.repo.ListSkills(context.Background(), wsID) + if err != nil { + t.Fatalf("list skills: %v", err) + } + for _, sk := range rows { + if sk.Slug == slug { + return sk + } + } + t.Fatalf("skill %q not found in workspace %q", slug, wsID) + return nil +} + +// routineByName finds a seeded/imported routine by name, failing if absent. +func routineByName(t *testing.T, e *testEnv, wsID, name string) *models.Routine { + t.Helper() + rows, err := e.repo.ListRoutines(context.Background(), wsID) + if err != nil { + t.Fatalf("list routines: %v", err) + } + for _, r := range rows { + if r.Name == name { + return r + } + } + t.Fatalf("routine %q not found in workspace %q", name, wsID) + return nil +} + +// projectByName finds a seeded/imported project by name, failing if absent. +func projectByName(t *testing.T, e *testEnv, wsID, name string) *models.Project { + t.Helper() + rows, err := e.repo.ListProjects(context.Background(), wsID) + if err != nil { + t.Fatalf("list projects: %v", err) + } + for _, p := range rows { + if p.Name == name { + return p + } + } + t.Fatalf("project %q not found in workspace %q", name, wsID) + return nil +} + +// assertStrings compares a []string against want by length and elements. A nil +// slice and an empty slice are equivalent here; pass want=nil to mean "no +// entries" regardless of which the production code returns. +func assertStrings(t *testing.T, label string, got, want []string) { + t.Helper() + if len(got) != len(want) { + t.Fatalf("%s: got %v (len %d), want %v (len %d)", label, got, len(got), want, len(want)) + } + for i := range want { + if got[i] != want[i] { + t.Errorf("%s[%d]: got %q, want %q", label, i, got[i], want[i]) + } + } +} + +// assertEqual fails with a labelled message when got != want. +func assertEqual[T comparable](t *testing.T, label string, got, want T) { + t.Helper() + if got != want { + t.Errorf("%s: got %v, want %v", label, got, want) + } +} diff --git a/apps/backend/internal/office/config/import_test.go b/apps/backend/internal/office/config/import_test.go new file mode 100644 index 0000000000..07ffe90c8e --- /dev/null +++ b/apps/backend/internal/office/config/import_test.go @@ -0,0 +1,453 @@ +package config + +import ( + "context" + "strings" + "testing" + + "github.com/kandev/kandev/internal/office/models" +) + +// TestApplyImport_CreatesEveryFieldFromBundle imports a bundle with every +// field populated into an empty workspace and asserts each one arrives in the +// DB. A field dropped from applyAgents/applySkills/applyRoutines/applyProjects +// is silent in production — no error, no panic, just missing configuration — +// so this is the test that has to catch it. +func TestApplyImport_CreatesEveryFieldFromBundle(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 4) + assertEqual(t, "updated count", result.UpdatedCount, 0) + + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "agent workspace", agent.WorkspaceID, testWorkspaceID) + assertEqual(t, "agent role", string(agent.Role), "cto") + assertEqual(t, "agent icon", agent.Icon, "🤖") + assertEqual(t, "agent status", agent.Status, models.AgentStatusIdle) + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", agent.MaxConcurrentSessions, 3) + assertEqual(t, "agent desired skills", agent.DesiredSkills, `["go","sql"]`) + assertEqual(t, "agent executor preference", agent.ExecutorPreference, "local_docker") + + skill := skillBySlug(t, env, testWorkspaceID, "code-review") + assertEqual(t, "skill workspace", skill.WorkspaceID, testWorkspaceID) + assertEqual(t, "skill name", skill.Name, "Code Review") + assertEqual(t, "skill description", skill.Description, "reviews diffs") + assertEqual(t, "skill source type", string(skill.SourceType), "inline") + assertEqual(t, "skill content", skill.Content, "# Code Review\n\nread the diff\n") + + routine := routineByName(t, env, testWorkspaceID, "standup") + assertEqual(t, "routine workspace", routine.WorkspaceID, testWorkspaceID) + assertEqual(t, "routine description", routine.Description, "daily standup") + assertEqual(t, "routine task template", routine.TaskTemplate, "summarise yesterday") + assertEqual(t, "routine concurrency", string(routine.ConcurrencyPolicy), "skip") + assertEqual(t, "routine status", routine.Status, "active") + + project := projectByName(t, env, testWorkspaceID, "apollo") + assertEqual(t, "project workspace", project.WorkspaceID, testWorkspaceID) + assertEqual(t, "project description", project.Description, "moon shot") + assertEqual(t, "project color", project.Color, "#ff00ff") + assertEqual(t, "project budget", project.BudgetCents, 99000) + assertEqual(t, "project repositories", project.Repositories, `["kdlbs/kandev"]`) + assertEqual(t, "project executor config", project.ExecutorConfig, `{"type":"local_docker"}`) +} + +// TestApplyImport_NameReferencesAreNotResolved pins the deliberate gap in the +// importer: the three cross-entity references in the DTO (agent reports_to, +// routine assignee_name, project lead_agent_name) name an entity but the +// import never resolves them to an ID. They are export-only fields today. +// Wiring resolution in must update this test. +func TestApplyImport_NameReferencesAreNotResolved(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + // Seed the referenced agent so a resolver would have something to find. + lead := seedAgent(t, env, testWorkspaceID, "grace") + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()); err != nil { + t.Fatalf("ApplyImport: %v", err) + } + + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "agent reports_to", agent.ReportsTo, "") + if agent.ReportsTo == lead.ID { + t.Error("reports_to resolution appears implemented; update this test") + } + assertEqual(t, "routine assignee", + routineByName(t, env, testWorkspaceID, "standup").AssigneeAgentProfileID, "") + assertEqual(t, "project lead", + projectByName(t, env, testWorkspaceID, "apollo").LeadAgentProfileID, "") +} + +// TestApplyImport_ProjectStatusIsForcedActive documents that a created project +// ignores the bundle's status and starts active, and that updating an existing +// project leaves its status untouched. +func TestApplyImport_ProjectStatusIsForcedActive(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + // fullBundle carries status "paused". + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()); err != nil { + t.Fatalf("ApplyImport create: %v", err) + } + assertEqual(t, "created project status", + projectByName(t, env, testWorkspaceID, "apollo").Status, models.ProjectStatusActive) + + bundle := fullBundle() + bundle.Projects[0].Status = "archived" + bundle.Projects[0].Description = "changed" + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle); err != nil { + t.Fatalf("ApplyImport update: %v", err) + } + updated := projectByName(t, env, testWorkspaceID, "apollo") + assertEqual(t, "updated project status", updated.Status, models.ProjectStatusActive) + assertEqual(t, "updated project description", updated.Description, "changed") +} + +// TestApplyImport_UpdatesExistingRowsInPlace changes every mutable field and +// asserts the update path carries each one through without creating a second +// row. +func TestApplyImport_UpdatesExistingRowsInPlace(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()); err != nil { + t.Fatalf("seed import: %v", err) + } + agentID := agentByName(t, env, testWorkspaceID, "ada").ID + skillID := skillBySlug(t, env, testWorkspaceID, "code-review").ID + routineID := routineByName(t, env, testWorkspaceID, "standup").ID + projectID := projectByName(t, env, testWorkspaceID, "apollo").ID + + changed := fullBundle() + changed.Agents[0].Role = "engineer" + changed.Agents[0].Icon = "🦾" + changed.Agents[0].BudgetMonthlyCents = 1 + changed.Agents[0].MaxConcurrentSessions = 9 + changed.Agents[0].DesiredSkills = `["rust"]` + changed.Agents[0].ExecutorPreference = "local_pc" + changed.Skills[0].Name = "Deep Review" + changed.Skills[0].Description = "reads everything" + changed.Skills[0].SourceType = "git" + changed.Skills[0].Content = "# Deep Review\n" + changed.Routines[0].Description = "weekly" + changed.Routines[0].TaskTemplate = "summarise the week" + changed.Routines[0].ConcurrencyPolicy = "queue" + changed.Projects[0].Description = "mars shot" + changed.Projects[0].Color = "#00ff00" + changed.Projects[0].BudgetCents = 1234 + changed.Projects[0].Repositories = `["kdlbs/other"]` + changed.Projects[0].ExecutorConfig = `{"type":"local_pc"}` + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, changed) + if err != nil { + t.Fatalf("ApplyImport update: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 0) + assertEqual(t, "updated count", result.UpdatedCount, 4) + + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "agent id stable", agent.ID, agentID) + assertEqual(t, "agent role", string(agent.Role), "engineer") + assertEqual(t, "agent icon", agent.Icon, "🦾") + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 1) + assertEqual(t, "agent max sessions", agent.MaxConcurrentSessions, 9) + assertEqual(t, "agent desired skills", agent.DesiredSkills, `["rust"]`) + assertEqual(t, "agent executor preference", agent.ExecutorPreference, "local_pc") + + skill := skillBySlug(t, env, testWorkspaceID, "code-review") + assertEqual(t, "skill id stable", skill.ID, skillID) + assertEqual(t, "skill name", skill.Name, "Deep Review") + assertEqual(t, "skill description", skill.Description, "reads everything") + assertEqual(t, "skill source type", string(skill.SourceType), "git") + assertEqual(t, "skill content", skill.Content, "# Deep Review\n") + + routine := routineByName(t, env, testWorkspaceID, "standup") + assertEqual(t, "routine id stable", routine.ID, routineID) + assertEqual(t, "routine description", routine.Description, "weekly") + assertEqual(t, "routine task template", routine.TaskTemplate, "summarise the week") + assertEqual(t, "routine concurrency", string(routine.ConcurrencyPolicy), "queue") + + project := projectByName(t, env, testWorkspaceID, "apollo") + assertEqual(t, "project id stable", project.ID, projectID) + assertEqual(t, "project description", project.Description, "mars shot") + assertEqual(t, "project color", project.Color, "#00ff00") + assertEqual(t, "project budget", project.BudgetCents, 1234) + assertEqual(t, "project repositories", project.Repositories, `["kdlbs/other"]`) + assertEqual(t, "project executor config", project.ExecutorConfig, `{"type":"local_pc"}`) +} + +// TestApplyImport_IsIdempotent imports the same bundle twice and asserts the +// second pass updates rather than duplicates, leaving exactly one row per +// entity with unchanged identity. +func TestApplyImport_IsIdempotent(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + bundle := fullBundle() + + first, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("first import: %v", err) + } + agentID := agentByName(t, env, testWorkspaceID, "ada").ID + + second, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("second import: %v", err) + } + assertEqual(t, "first created", first.CreatedCount, 4) + assertEqual(t, "second created", second.CreatedCount, 0) + assertEqual(t, "second updated", second.UpdatedCount, 4) + + agents, err := env.repo.ListAgentInstances(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "agent row count", len(agents), 1) + assertEqual(t, "agent id unchanged", agents[0].ID, agentID) + + skills, err := env.repo.ListSkills(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list skills: %v", err) + } + assertEqual(t, "skill row count", len(skills), 1) + + routines, err := env.repo.ListRoutines(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list routines: %v", err) + } + assertEqual(t, "routine row count", len(routines), 1) + + projects, err := env.repo.ListProjects(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list projects: %v", err) + } + assertEqual(t, "project row count", len(projects), 1) +} + +// TestApplyImport_MatchesExistingByNameNotIdentity proves the dedup key: an +// agent already present under the same name is updated (not duplicated) even +// though the bundle carries no ID, and skills key on slug rather than name. +func TestApplyImport_MatchesExistingByNameNotIdentity(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + existingAgent := seedAgent(t, env, testWorkspaceID, "ada") + existingSkill := seedSkill(t, env, testWorkspaceID, "code-review") + + bundle := fullBundle() + // Same slug, different display name: still an update. + bundle.Skills[0].Name = "Renamed Review" + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 2) // routine + project + assertEqual(t, "updated count", result.UpdatedCount, 2) // agent + skill + + assertEqual(t, "agent id reused", agentByName(t, env, testWorkspaceID, "ada").ID, existingAgent.ID) + skill := skillBySlug(t, env, testWorkspaceID, "code-review") + assertEqual(t, "skill id reused", skill.ID, existingSkill.ID) + assertEqual(t, "skill renamed", skill.Name, "Renamed Review") +} + +// TestApplyImport_IgnoresOtherWorkspaces confirms an import scoped to one +// workspace neither reads nor rewrites a same-named row in another. +func TestApplyImport_IgnoresOtherWorkspaces(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + foreign := seedAgent(t, env, "ws-other", "ada") + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 4) + + mine := agentByName(t, env, testWorkspaceID, "ada") + if mine.ID == foreign.ID { + t.Fatal("import adopted another workspace's agent row") + } + other := agentByName(t, env, "ws-other", "ada") + assertEqual(t, "foreign agent icon untouched", other.Icon, "🛠") + assertEqual(t, "foreign agent budget untouched", other.BudgetMonthlyCents, 100) +} + +func TestApplyImport_EmptyBundleIsANoOp(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + + result, err := env.svc.ApplyImport(ctx, testWorkspaceID, &ConfigBundle{}) + if err != nil { + t.Fatalf("ApplyImport: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 0) + assertEqual(t, "updated count", result.UpdatedCount, 0) + + agents, err := env.repo.ListAgentInstances(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "existing rows survive", len(agents), 1) +} + +// TestApplyImport_LogsActivityWithCounts asserts the audit entry the import +// emits, including the counts embedded in its details string. +func TestApplyImport_LogsActivityWithCounts(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + + if _, err := env.svc.ApplyImport(ctx, testWorkspaceID, fullBundle()); err != nil { + t.Fatalf("ApplyImport: %v", err) + } + + if len(env.activity.entries) != 1 { + t.Fatalf("activity entries: got %d, want 1", len(env.activity.entries)) + } + got := env.activity.entries[0] + assertEqual(t, "activity workspace", got.wsID, testWorkspaceID) + assertEqual(t, "activity actor type", got.actorType, "user") + assertEqual(t, "activity action", got.action, "config_imported") + assertEqual(t, "activity target type", got.targetType, "workspace") + assertEqual(t, "activity target id", got.targetID, testWorkspaceID) + assertEqual(t, "activity details", got.details, "created=3 updated=1") +} + +// TestApplyImport_WrapsRepositoryErrors breaks one entity's table at a time so +// each apply stage fails on its own, and asserts the stage names itself in the +// wrapped error rather than surfacing a bare SQL message. +// +// wantAgentSurvives records the other half of the contract: the import is not +// wrapped in a transaction, so rows written by stages that ran before the +// failure stay in the DB. Adding a transaction here should flip these. +func TestApplyImport_WrapsRepositoryErrors(t *testing.T) { + tests := []struct { + name string + dropTable string + wantPrefix string + wantAgentSurvives bool + }{ + {"agents", "agent_profiles", "apply agents:", false}, + {"skills", "office_skills", "apply skills:", true}, + {"routines", "office_routines", "apply routines:", true}, + {"projects", "office_projects", "apply projects:", true}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, tc.dropTable) + + result, err := env.svc.ApplyImport(context.Background(), testWorkspaceID, fullBundle()) + if err == nil { + t.Fatalf("expected an error after dropping %s, got nil", tc.dropTable) + } + if result != nil { + t.Errorf("result should be nil on error, got %+v", result) + } + if !strings.HasPrefix(err.Error(), tc.wantPrefix) { + t.Errorf("error %q does not start with %q", err.Error(), tc.wantPrefix) + } + if len(env.activity.entries) != 0 { + t.Errorf("failed import should not log activity, got %d entries", + len(env.activity.entries)) + } + if !tc.wantAgentSurvives { + return + } + // The agent stage ran to completion before the failure. Its row + // is committed and is not rolled back. + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "partially imported agent survives", agent.Name, "ada") + }) + } +} + +// TestPreviewImport_ClassifiesCreatesAndUpdates checks every entity type is +// split against existing DB state on the right key, and that preview writes +// nothing. +func TestPreviewImport_ClassifiesCreatesAndUpdates(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + seedSkill(t, env, testWorkspaceID, "code-review") + seedRoutine(t, env, testWorkspaceID, "standup") + seedProject(t, env, testWorkspaceID, "apollo") + + bundle := fullBundle() + bundle.Agents = append(bundle.Agents, AgentConfig{Name: "grace"}) + bundle.Skills = append(bundle.Skills, SkillConfig{Slug: "triage"}) + bundle.Routines = append(bundle.Routines, RoutineConfig{Name: "retro"}) + bundle.Projects = append(bundle.Projects, ProjectConfig{Name: "gemini"}) + + preview, err := env.svc.PreviewImport(ctx, testWorkspaceID, bundle) + if err != nil { + t.Fatalf("PreviewImport: %v", err) + } + + assertStrings(t, "agents updated", preview.Agents.Updated, []string{"ada"}) + assertStrings(t, "agents created", preview.Agents.Created, []string{"grace"}) + assertStrings(t, "skills updated", preview.Skills.Updated, []string{"code-review"}) + assertStrings(t, "skills created", preview.Skills.Created, []string{"triage"}) + assertStrings(t, "routines updated", preview.Routines.Updated, []string{"standup"}) + assertStrings(t, "routines created", preview.Routines.Created, []string{"retro"}) + assertStrings(t, "projects updated", preview.Projects.Updated, []string{"apollo"}) + assertStrings(t, "projects created", preview.Projects.Created, []string{"gemini"}) + + // PreviewImport never populates deletions — that is IncomingDiff's job. + assertStrings(t, "agents deleted", preview.Agents.Deleted, nil) + assertStrings(t, "routines deleted", preview.Routines.Deleted, nil) + + agents, err := env.repo.ListAgentInstances(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertEqual(t, "preview wrote no rows", len(agents), 1) +} + +func TestPreviewImport_EmptyBundleYieldsEmptyDiffs(t *testing.T) { + env := newTestEnv(t) + seedAgent(t, env, testWorkspaceID, "ada") + + preview, err := env.svc.PreviewImport(context.Background(), testWorkspaceID, &ConfigBundle{}) + if err != nil { + t.Fatalf("PreviewImport: %v", err) + } + assertStrings(t, "agents created", preview.Agents.Created, nil) + assertStrings(t, "agents updated", preview.Agents.Updated, nil) + assertStrings(t, "skills created", preview.Skills.Created, nil) + assertStrings(t, "projects created", preview.Projects.Created, nil) +} + +// TestPreviewImport_ScopesToWorkspace confirms an existing row in another +// workspace does not turn a create into an update. +func TestPreviewImport_ScopesToWorkspace(t *testing.T) { + env := newTestEnv(t) + seedAgent(t, env, "ws-other", "ada") + + preview, err := env.svc.PreviewImport(context.Background(), testWorkspaceID, fullBundle()) + if err != nil { + t.Fatalf("PreviewImport: %v", err) + } + assertStrings(t, "agents created", preview.Agents.Created, []string{"ada"}) + assertStrings(t, "agents updated", preview.Agents.Updated, nil) +} + +func TestPreviewImport_PropagatesRepositoryErrors(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + + preview, err := env.svc.PreviewImport(context.Background(), testWorkspaceID, fullBundle()) + if err == nil { + t.Fatal("expected an error from a closed database, got nil") + } + if preview != nil { + t.Errorf("preview should be nil on error, got %+v", preview) + } +} diff --git a/apps/backend/internal/office/config/service_test.go b/apps/backend/internal/office/config/service_test.go new file mode 100644 index 0000000000..4def22b897 --- /dev/null +++ b/apps/backend/internal/office/config/service_test.go @@ -0,0 +1,411 @@ +package config + +import ( + "archive/zip" + "bytes" + "context" + "io" + "sort" + "testing" + + "gopkg.in/yaml.v3" + + "github.com/kandev/kandev/internal/office/models" +) + +func TestNewConfigService_WiresDependencies(t *testing.T) { + env := newTestEnv(t) + + if env.svc.repo != env.repo { + t.Error("repo not wired") + } + if env.svc.cfgLoader != env.loader { + t.Error("config loader not wired") + } + if env.svc.cfgWriter != env.writer { + t.Error("config writer not wired") + } + if env.svc.logger == nil { + t.Error("logger not wired") + } + if env.svc.activity == nil { + t.Error("activity logger not wired") + } +} + +// seedFullDBState writes one of each entity with every exportable column set, +// plus a second agent used as the target of the three name references. +func seedFullDBState(t *testing.T, env *testEnv, wsID string) *models.AgentInstance { + t.Helper() + ctx := context.Background() + + lead := seedAgent(t, env, wsID, "grace") + + agent := &models.AgentInstance{ + WorkspaceID: wsID, + Name: "ada", + Role: models.AgentRole("cto"), + Icon: "🤖", + Status: models.AgentStatusIdle, + ReportsTo: lead.ID, + BudgetMonthlyCents: 4200, + MaxConcurrentSessions: 3, + DesiredSkills: `["go","sql"]`, + ExecutorPreference: "local_docker", + } + if err := env.repo.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + + skill := &models.Skill{ + WorkspaceID: wsID, + Name: "Code Review", + Slug: "code-review", + Description: "reviews diffs", + SourceType: models.SkillSourceType("inline"), + Content: "# Code Review\n", + } + if err := env.repo.CreateSkill(ctx, skill); err != nil { + t.Fatalf("create skill: %v", err) + } + + routine := &models.Routine{ + WorkspaceID: wsID, + Name: "standup", + Description: "daily standup", + TaskTemplate: "summarise yesterday", + AssigneeAgentProfileID: lead.ID, + Status: "active", + ConcurrencyPolicy: models.RoutineConcurrencyPolicy("skip"), + } + if err := env.repo.CreateRoutine(ctx, routine); err != nil { + t.Fatalf("create routine: %v", err) + } + + project := &models.Project{ + WorkspaceID: wsID, + Name: "apollo", + Description: "moon shot", + Status: models.ProjectStatus("paused"), + LeadAgentProfileID: lead.ID, + Color: "#ff00ff", + BudgetCents: 99000, + Repositories: `["kdlbs/kandev"]`, + ExecutorConfig: `{"type":"local_docker"}`, + } + if err := env.repo.CreateProject(ctx, project); err != nil { + t.Fatalf("create project: %v", err) + } + return lead +} + +// TestExportBundle_CarriesEveryField exports fully-populated DB rows and +// asserts each column reaches the bundle, including the three ID→name +// references the exporter resolves. +func TestExportBundle_CarriesEveryField(t *testing.T) { + env := newTestEnv(t) + seedFullDBState(t, env, testWorkspaceID) + + bundle, err := env.svc.ExportBundle(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ExportBundle: %v", err) + } + + assertEqual(t, "settings name", bundle.Settings.Name, testWorkspaceID) + + var ada *AgentConfig + for i := range bundle.Agents { + if bundle.Agents[i].Name == "ada" { + ada = &bundle.Agents[i] + } + } + if ada == nil { + t.Fatalf("agent ada missing from export (%d agents)", len(bundle.Agents)) + } + assertEqual(t, "agent role", ada.Role, "cto") + assertEqual(t, "agent icon", ada.Icon, "🤖") + assertEqual(t, "agent reports_to resolved to name", ada.ReportsTo, "grace") + assertEqual(t, "agent budget", ada.BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", ada.MaxConcurrentSessions, 3) + assertEqual(t, "agent desired skills", ada.DesiredSkills, `["go","sql"]`) + assertEqual(t, "agent executor preference", ada.ExecutorPreference, "local_docker") + + if len(bundle.Skills) != 1 { + t.Fatalf("skills: got %d, want 1", len(bundle.Skills)) + } + assertEqual(t, "skill name", bundle.Skills[0].Name, "Code Review") + assertEqual(t, "skill slug", bundle.Skills[0].Slug, "code-review") + assertEqual(t, "skill description", bundle.Skills[0].Description, "reviews diffs") + assertEqual(t, "skill source type", bundle.Skills[0].SourceType, "inline") + assertEqual(t, "skill content", bundle.Skills[0].Content, "# Code Review\n") + + if len(bundle.Routines) != 1 { + t.Fatalf("routines: got %d, want 1", len(bundle.Routines)) + } + assertEqual(t, "routine name", bundle.Routines[0].Name, "standup") + assertEqual(t, "routine description", bundle.Routines[0].Description, "daily standup") + assertEqual(t, "routine task template", bundle.Routines[0].TaskTemplate, "summarise yesterday") + assertEqual(t, "routine assignee resolved to name", bundle.Routines[0].AssigneeName, "grace") + assertEqual(t, "routine concurrency", bundle.Routines[0].ConcurrencyPolicy, "skip") + + if len(bundle.Projects) != 1 { + t.Fatalf("projects: got %d, want 1", len(bundle.Projects)) + } + assertEqual(t, "project name", bundle.Projects[0].Name, "apollo") + assertEqual(t, "project description", bundle.Projects[0].Description, "moon shot") + assertEqual(t, "project status", bundle.Projects[0].Status, "paused") + assertEqual(t, "project color", bundle.Projects[0].Color, "#ff00ff") + assertEqual(t, "project budget", bundle.Projects[0].BudgetCents, 99000) + assertEqual(t, "project repositories", bundle.Projects[0].Repositories, `["kdlbs/kandev"]`) + assertEqual(t, "project executor config", bundle.Projects[0].ExecutorConfig, + `{"type":"local_docker"}`) + assertEqual(t, "project lead resolved to name", bundle.Projects[0].LeadAgentName, "grace") +} + +// TestExportBundle_UnresolvableReferencesBecomeEmpty covers the other branch of +// the ID→name lookup: an ID with no matching agent exports as an empty string +// rather than leaking the raw ID. +func TestExportBundle_UnresolvableReferencesBecomeEmpty(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + + agent := &models.AgentInstance{ + WorkspaceID: testWorkspaceID, Name: "ada", ReportsTo: "no-such-agent", + } + if err := env.repo.CreateAgentInstance(ctx, agent); err != nil { + t.Fatalf("create agent: %v", err) + } + routine := &models.Routine{ + WorkspaceID: testWorkspaceID, Name: "standup", AssigneeAgentProfileID: "no-such-agent", + } + if err := env.repo.CreateRoutine(ctx, routine); err != nil { + t.Fatalf("create routine: %v", err) + } + project := &models.Project{ + WorkspaceID: testWorkspaceID, Name: "apollo", LeadAgentProfileID: "no-such-agent", + } + if err := env.repo.CreateProject(ctx, project); err != nil { + t.Fatalf("create project: %v", err) + } + + bundle, err := env.svc.ExportBundle(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ExportBundle: %v", err) + } + assertEqual(t, "agent reports_to", bundle.Agents[0].ReportsTo, "") + assertEqual(t, "routine assignee", bundle.Routines[0].AssigneeName, "") + assertEqual(t, "project lead", bundle.Projects[0].LeadAgentName, "") +} + +// TestExportBundle_LeaksOtherWorkspaces_KnownDefect records a suspected bug, +// not an endorsed contract. Every export helper calls ListX(ctx, "") and so +// lists across all workspaces; the workspace ID argument only labels the +// settings block. PreviewImport on the same service *is* scoped (see +// TestPreviewImport_ScopesToWorkspace), so the two disagree, and under +// enabled auth a config export or zip for one workspace can carry another +// user's agents, routines, projects and skill content. +// +// This is reported separately rather than fixed here: this package's tests +// were added under a test-only mandate. Scoping the export is the fix, and it +// must delete or invert this test — that failure is the intended signal. +func TestExportBundle_LeaksOtherWorkspaces_KnownDefect(t *testing.T) { + env := newTestEnv(t) + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, "ws-other", "grace") + seedSkill(t, env, "ws-other", "triage") + seedRoutine(t, env, "ws-other", "retro") + seedProject(t, env, "ws-other", "gemini") + + bundle, err := env.svc.ExportBundle(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ExportBundle: %v", err) + } + + names := agentBundleNames(bundle.Agents) + sort.Strings(names) + assertStrings(t, "agents across workspaces", names, []string{"ada", "grace"}) + assertStrings(t, "skills across workspaces", skillBundleSlugs(bundle.Skills), []string{"triage"}) + assertStrings(t, "routines across workspaces", + routineBundleNames(bundle.Routines), []string{"retro"}) + assertStrings(t, "projects across workspaces", + projectBundleNames(bundle.Projects), []string{"gemini"}) +} + +func TestExportBundle_EmptyDatabase(t *testing.T) { + env := newTestEnv(t) + + bundle, err := env.svc.ExportBundle(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ExportBundle: %v", err) + } + assertEqual(t, "settings name", bundle.Settings.Name, testWorkspaceID) + assertEqual(t, "agents", len(bundle.Agents), 0) + assertEqual(t, "skills", len(bundle.Skills), 0) + assertEqual(t, "routines", len(bundle.Routines), 0) + assertEqual(t, "projects", len(bundle.Projects), 0) +} + +// TestExportBundle_WrapsRepositoryErrors drops one table at a time and asserts +// each export stage names its own failure. +func TestExportBundle_WrapsRepositoryErrors(t *testing.T) { + tests := []struct { + name string + dropTable string + wantIn string + }{ + {"agents", "agent_profiles", "list agents:"}, + {"skills", "office_skills", "list skills:"}, + {"routines", "office_routines", "list routines:"}, + {"projects", "office_projects", "list projects:"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, tc.dropTable) + + bundle, err := env.svc.ExportBundle(context.Background(), testWorkspaceID) + if err == nil { + t.Fatalf("expected an error after dropping %s, got nil", tc.dropTable) + } + if bundle != nil { + t.Errorf("bundle should be nil on error, got %+v", bundle) + } + if !bytes.Contains([]byte(err.Error()), []byte(tc.wantIn)) { + t.Errorf("error %q does not contain %q", err.Error(), tc.wantIn) + } + }) + } +} + +// readZip returns every entry of a zip archive keyed by name. +func readZip(t *testing.T, r io.Reader) map[string][]byte { + t.Helper() + data, err := io.ReadAll(r) + if err != nil { + t.Fatalf("read zip bytes: %v", err) + } + zr, err := zip.NewReader(bytes.NewReader(data), int64(len(data))) + if err != nil { + t.Fatalf("open zip: %v", err) + } + out := make(map[string][]byte, len(zr.File)) + for _, f := range zr.File { + rc, openErr := f.Open() + if openErr != nil { + t.Fatalf("open zip entry %s: %v", f.Name, openErr) + } + body, readErr := io.ReadAll(rc) + _ = rc.Close() + if readErr != nil { + t.Fatalf("read zip entry %s: %v", f.Name, readErr) + } + out[f.Name] = body + } + return out +} + +// TestBundleToZip_EveryFieldSurvivesTheArchive marshals a fully-populated +// bundle and unmarshals each entry back into its DTO, so a field lost in the +// YAML layer fails here. +func TestBundleToZip_EveryFieldSurvivesTheArchive(t *testing.T) { + want := fullBundle() + + r, err := bundleToZip(want) + if err != nil { + t.Fatalf("bundleToZip: %v", err) + } + entries := readZip(t, r) + + if len(entries) != 5 { + t.Fatalf("zip entries: got %d (%v), want 5", len(entries), entries) + } + + var settings SettingsConfig + unmarshalEntry(t, entries, ".kandev/kandev.yml", &settings) + assertEqual(t, "settings name", settings.Name, want.Settings.Name) + assertEqual(t, "settings description", settings.Description, want.Settings.Description) + + var agent AgentConfig + unmarshalEntry(t, entries, ".kandev/agents/ada.yml", &agent) + assertEqual(t, "agent", agent, want.Agents[0]) + + var skill SkillConfig + unmarshalEntry(t, entries, ".kandev/skills/code-review.yml", &skill) + assertEqual(t, "skill", skill, want.Skills[0]) + + var routine RoutineConfig + unmarshalEntry(t, entries, ".kandev/routines/standup.yml", &routine) + assertEqual(t, "routine", routine, want.Routines[0]) + + var project ProjectConfig + unmarshalEntry(t, entries, ".kandev/projects/apollo.yml", &project) + assertEqual(t, "project", project, want.Projects[0]) +} + +func unmarshalEntry(t *testing.T, entries map[string][]byte, name string, out interface{}) { + t.Helper() + body, ok := entries[name] + if !ok { + t.Fatalf("zip entry %q missing", name) + } + if err := yaml.Unmarshal(body, out); err != nil { + t.Fatalf("unmarshal %s: %v", name, err) + } +} + +func TestBundleToZip_EmptyBundleStillCarriesSettings(t *testing.T) { + r, err := bundleToZip(&ConfigBundle{}) + if err != nil { + t.Fatalf("bundleToZip: %v", err) + } + entries := readZip(t, r) + assertEqual(t, "zip entries", len(entries), 1) + if _, ok := entries[".kandev/kandev.yml"]; !ok { + t.Errorf("settings entry missing, got %v", entries) + } +} + +// TestExportZip_ReflectsDatabaseState wires ExportBundle and bundleToZip +// together over real rows. +func TestExportZip_ReflectsDatabaseState(t *testing.T) { + env := newTestEnv(t) + seedFullDBState(t, env, testWorkspaceID) + + r, err := env.svc.ExportZip(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ExportZip: %v", err) + } + entries := readZip(t, r) + + for _, name := range []string{ + ".kandev/kandev.yml", + ".kandev/agents/ada.yml", + ".kandev/agents/grace.yml", + ".kandev/skills/code-review.yml", + ".kandev/routines/standup.yml", + ".kandev/projects/apollo.yml", + } { + if _, ok := entries[name]; !ok { + t.Errorf("zip entry %q missing", name) + } + } + + var agent AgentConfig + unmarshalEntry(t, entries, ".kandev/agents/ada.yml", &agent) + assertEqual(t, "agent role", agent.Role, "cto") + assertEqual(t, "agent reports_to", agent.ReportsTo, "grace") + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) +} + +func TestExportZip_PropagatesExportError(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + + r, err := env.svc.ExportZip(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error from a closed database, got nil") + } + if r != nil { + t.Error("reader should be nil on error") + } +} diff --git a/apps/backend/internal/office/config/sync_helpers_test.go b/apps/backend/internal/office/config/sync_helpers_test.go new file mode 100644 index 0000000000..9336b2f401 --- /dev/null +++ b/apps/backend/internal/office/config/sync_helpers_test.go @@ -0,0 +1,257 @@ +package config + +import ( + "context" + "sort" + "testing" +) + +// TestDiffBundles_ClassifiesEveryEntityType feeds an incoming bundle that +// creates, updates and deletes one of each entity type, and asserts each +// bucket. Inverting any of the four membership checks flips a create into an +// update here. +func TestDiffBundles_ClassifiesEveryEntityType(t *testing.T) { + existing := &ConfigBundle{ + Agents: []AgentConfig{{Name: "ada"}, {Name: "gone-agent"}}, + Skills: []SkillConfig{{Slug: "code-review"}, {Slug: "gone-skill"}}, + Routines: []RoutineConfig{{Name: "standup"}, {Name: "gone-routine"}}, + Projects: []ProjectConfig{{Name: "apollo"}, {Name: "gone-project"}}, + } + incoming := &ConfigBundle{ + Agents: []AgentConfig{{Name: "ada"}, {Name: "new-agent"}}, + Skills: []SkillConfig{{Slug: "code-review"}, {Slug: "new-skill"}}, + Routines: []RoutineConfig{{Name: "standup"}, {Name: "new-routine"}}, + Projects: []ProjectConfig{{Name: "apollo"}, {Name: "new-project"}}, + } + + preview := diffBundles(incoming, existing) + + assertStrings(t, "agents updated", preview.Agents.Updated, []string{"ada"}) + assertStrings(t, "agents created", preview.Agents.Created, []string{"new-agent"}) + assertStrings(t, "agents deleted", preview.Agents.Deleted, []string{"gone-agent"}) + + assertStrings(t, "skills updated", preview.Skills.Updated, []string{"code-review"}) + assertStrings(t, "skills created", preview.Skills.Created, []string{"new-skill"}) + assertStrings(t, "skills deleted", preview.Skills.Deleted, []string{"gone-skill"}) + + assertStrings(t, "routines updated", preview.Routines.Updated, []string{"standup"}) + assertStrings(t, "routines created", preview.Routines.Created, []string{"new-routine"}) + assertStrings(t, "routines deleted", preview.Routines.Deleted, []string{"gone-routine"}) + + assertStrings(t, "projects updated", preview.Projects.Updated, []string{"apollo"}) + assertStrings(t, "projects created", preview.Projects.Created, []string{"new-project"}) + assertStrings(t, "projects deleted", preview.Projects.Deleted, []string{"gone-project"}) +} + +// TestDiffBundles_SkillsKeyOnSlug guards the one entity that does not key on +// Name: a skill whose slug matches but whose name changed is an update, and a +// skill whose name matches but whose slug changed is a create plus a delete. +func TestDiffBundles_SkillsKeyOnSlug(t *testing.T) { + existing := &ConfigBundle{Skills: []SkillConfig{{Name: "Code Review", Slug: "code-review"}}} + + renamed := diffBundles( + &ConfigBundle{Skills: []SkillConfig{{Name: "Deep Review", Slug: "code-review"}}}, existing) + assertStrings(t, "renamed updated", renamed.Skills.Updated, []string{"code-review"}) + assertStrings(t, "renamed deleted", renamed.Skills.Deleted, nil) + + reslugged := diffBundles( + &ConfigBundle{Skills: []SkillConfig{{Name: "Code Review", Slug: "deep-review"}}}, existing) + assertStrings(t, "reslugged created", reslugged.Skills.Created, []string{"deep-review"}) + assertStrings(t, "reslugged deleted", reslugged.Skills.Deleted, []string{"code-review"}) +} + +func TestDiffBundles_EmptySides(t *testing.T) { + full := fullBundle() + + fromEmpty := diffBundles(full, &ConfigBundle{}) + assertStrings(t, "everything created", fromEmpty.Agents.Created, []string{"ada"}) + assertStrings(t, "nothing deleted", fromEmpty.Agents.Deleted, nil) + + toEmpty := diffBundles(&ConfigBundle{}, full) + assertStrings(t, "nothing created", toEmpty.Agents.Created, nil) + assertStrings(t, "everything deleted", toEmpty.Agents.Deleted, []string{"ada"}) + assertStrings(t, "skills deleted", toEmpty.Skills.Deleted, []string{"code-review"}) + assertStrings(t, "routines deleted", toEmpty.Routines.Deleted, []string{"standup"}) + assertStrings(t, "projects deleted", toEmpty.Projects.Deleted, []string{"apollo"}) + + both := diffBundles(&ConfigBundle{}, &ConfigBundle{}) + assertStrings(t, "empty created", both.Agents.Created, nil) + assertStrings(t, "empty updated", both.Agents.Updated, nil) + assertStrings(t, "empty deleted", both.Agents.Deleted, nil) +} + +// TestAppendDeletions_ReportsRowsMissingFromBundle covers all four entity +// types and confirms the created/updated buckets are left alone. +func TestAppendDeletions_ReportsRowsMissingFromBundle(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, testWorkspaceID, "gone-agent") + seedSkill(t, env, testWorkspaceID, "code-review") + seedSkill(t, env, testWorkspaceID, "gone-skill") + seedRoutine(t, env, testWorkspaceID, "standup") + seedRoutine(t, env, testWorkspaceID, "gone-routine") + seedProject(t, env, testWorkspaceID, "apollo") + seedProject(t, env, testWorkspaceID, "gone-project") + + preview := &ImportPreview{} + preview.Agents.Created = []string{"pre-existing"} + + if err := env.svc.appendDeletions(ctx, testWorkspaceID, fullBundle(), preview); err != nil { + t.Fatalf("appendDeletions: %v", err) + } + + assertStrings(t, "agents deleted", preview.Agents.Deleted, []string{"gone-agent"}) + assertStrings(t, "skills deleted", preview.Skills.Deleted, []string{"gone-skill"}) + assertStrings(t, "routines deleted", preview.Routines.Deleted, []string{"gone-routine"}) + assertStrings(t, "projects deleted", preview.Projects.Deleted, []string{"gone-project"}) + assertStrings(t, "created bucket untouched", preview.Agents.Created, []string{"pre-existing"}) +} + +// TestAppendDeletions_IgnoresOtherWorkspaces makes sure a row that only exists +// in another workspace is never proposed for deletion. +func TestAppendDeletions_IgnoresOtherWorkspaces(t *testing.T) { + env := newTestEnv(t) + seedAgent(t, env, "ws-other", "foreign") + + preview := &ImportPreview{} + if err := env.svc.appendDeletions( + context.Background(), testWorkspaceID, fullBundle(), preview); err != nil { + t.Fatalf("appendDeletions: %v", err) + } + assertStrings(t, "agents deleted", preview.Agents.Deleted, nil) +} + +func TestAppendDeletions_PropagatesRepositoryErrors(t *testing.T) { + for _, table := range []string{ + "agent_profiles", "office_skills", "office_routines", "office_projects", + } { + t.Run(table, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, table) + + err := env.svc.appendDeletions( + context.Background(), testWorkspaceID, fullBundle(), &ImportPreview{}) + if err == nil { + t.Fatalf("expected an error after dropping %s, got nil", table) + } + }) + } +} + +// TestDeleteRowsMissingFromBundle_RemovesOnlyAbsentRows seeds two rows per +// entity type, keeps one of each in the bundle, and asserts exactly the other +// is gone. +func TestDeleteRowsMissingFromBundle_RemovesOnlyAbsentRows(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, testWorkspaceID, "gone-agent") + seedSkill(t, env, testWorkspaceID, "code-review") + seedSkill(t, env, testWorkspaceID, "gone-skill") + seedRoutine(t, env, testWorkspaceID, "standup") + seedRoutine(t, env, testWorkspaceID, "gone-routine") + seedProject(t, env, testWorkspaceID, "apollo") + seedProject(t, env, testWorkspaceID, "gone-project") + + if err := env.svc.deleteRowsMissingFromBundle(ctx, testWorkspaceID, fullBundle()); err != nil { + t.Fatalf("deleteRowsMissingFromBundle: %v", err) + } + + assertRemainingNames(t, env, testWorkspaceID, []string{"ada"}, []string{"code-review"}, + []string{"standup"}, []string{"apollo"}) +} + +// TestDeleteRowsMissingFromBundle_EmptyBundleClearsWorkspace is the +// destructive end of the range: nothing on disk means nothing in the DB. +func TestDeleteRowsMissingFromBundle_EmptyBundleClearsWorkspace(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + seedSkill(t, env, testWorkspaceID, "code-review") + seedRoutine(t, env, testWorkspaceID, "standup") + seedProject(t, env, testWorkspaceID, "apollo") + + if err := env.svc.deleteRowsMissingFromBundle(ctx, testWorkspaceID, &ConfigBundle{}); err != nil { + t.Fatalf("deleteRowsMissingFromBundle: %v", err) + } + + assertRemainingNames(t, env, testWorkspaceID, nil, nil, nil, nil) +} + +// TestDeleteRowsMissingFromBundle_LeavesOtherWorkspacesAlone is the guard that +// keeps an FS sync of one workspace from wiping another. +func TestDeleteRowsMissingFromBundle_LeavesOtherWorkspacesAlone(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, "ws-other", "foreign-agent") + seedSkill(t, env, "ws-other", "foreign-skill") + seedRoutine(t, env, "ws-other", "foreign-routine") + seedProject(t, env, "ws-other", "foreign-project") + + if err := env.svc.deleteRowsMissingFromBundle(ctx, testWorkspaceID, &ConfigBundle{}); err != nil { + t.Fatalf("deleteRowsMissingFromBundle: %v", err) + } + + assertRemainingNames(t, env, "ws-other", []string{"foreign-agent"}, []string{"foreign-skill"}, + []string{"foreign-routine"}, []string{"foreign-project"}) +} + +func TestDeleteRowsMissingFromBundle_PropagatesRepositoryErrors(t *testing.T) { + for _, table := range []string{ + "agent_profiles", "office_skills", "office_routines", "office_projects", + } { + t.Run(table, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, table) + + err := env.svc.deleteRowsMissingFromBundle( + context.Background(), testWorkspaceID, fullBundle()) + if err == nil { + t.Fatalf("expected an error after dropping %s, got nil", table) + } + }) + } +} + +// assertRemainingNames compares the workspace's surviving rows, sorted, with +// the expected names for each entity type. +func assertRemainingNames( + t *testing.T, env *testEnv, wsID string, agents, skills, routines, projects []string, +) { + t.Helper() + ctx := context.Background() + + gotAgents, err := env.repo.ListAgentInstances(ctx, wsID) + if err != nil { + t.Fatalf("list agents: %v", err) + } + assertSorted(t, "agents", agentNames(gotAgents), agents) + + gotSkills, err := env.repo.ListSkills(ctx, wsID) + if err != nil { + t.Fatalf("list skills: %v", err) + } + assertSorted(t, "skills", skillSlugs(gotSkills), skills) + + gotRoutines, err := env.repo.ListRoutines(ctx, wsID) + if err != nil { + t.Fatalf("list routines: %v", err) + } + assertSorted(t, "routines", routineNames(gotRoutines), routines) + + gotProjects, err := env.repo.ListProjects(ctx, wsID) + if err != nil { + t.Fatalf("list projects: %v", err) + } + assertSorted(t, "projects", projectNames(gotProjects), projects) +} + +func assertSorted(t *testing.T, label string, got, want []string) { + t.Helper() + sort.Strings(got) + if want == nil { + want = []string{} + } + assertStrings(t, label, got, want) +} diff --git a/apps/backend/internal/office/config/sync_test.go b/apps/backend/internal/office/config/sync_test.go new file mode 100644 index 0000000000..a31e92e955 --- /dev/null +++ b/apps/backend/internal/office/config/sync_test.go @@ -0,0 +1,573 @@ +package config + +import ( + "context" + "os" + "path/filepath" + "sort" + "strings" + "testing" + + "github.com/kandev/kandev/internal/common/logger" +) + +const fullAgentYAML = `name: ada +role: cto +icon: "🤖" +reports_to: grace +permissions: '{"hire_agents":true}' +budget_monthly_cents: 4200 +max_concurrent_sessions: 3 +desired_skills: '["go","sql"]' +executor_preference: local_docker +` + +const fullRoutineYAML = `name: standup +description: daily standup +task_template: summarise yesterday +assignee_name: grace +status: paused +concurrency_policy: skip +` + +const fullProjectYAML = `name: apollo +description: moon shot +status: paused +color: "#ff00ff" +budget_cents: 99000 +repositories: '["kdlbs/kandev"]' +executor_config: '{"type":"local_docker"}' +lead_agent_name: grace +` + +// seedFullFSState writes one fully-populated file per entity type into the +// on-disk "default" workspace. +func seedFullFSState(t *testing.T, env *testEnv) { + t.Helper() + env.writeFSAgent(t, "ada", fullAgentYAML) + env.writeFSSkill(t, "code-review", "# Code Review\n\nread the diff\n") + env.writeFSRoutine(t, "standup", fullRoutineYAML) + env.writeFSProject(t, "apollo", fullProjectYAML) +} + +func sortedCopy(in []string) []string { + out := append([]string(nil), in...) + sort.Strings(out) + return out +} + +// TestScanFilesystem_CarriesEveryFieldFromDisk is the FS→bundle half of the +// field-drop guard: every value written to disk must arrive in the bundle. +func TestScanFilesystem_CarriesEveryFieldFromDisk(t *testing.T) { + env := newTestEnv(t) + seedFullFSState(t, env) + + bundle, parseErrs, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + if len(parseErrs) != 0 { + t.Fatalf("unexpected parse errors: %+v", parseErrs) + } + + // The scan always reads the "default" FS workspace, regardless of the + // DB workspace ID passed in. + assertEqual(t, "settings name", bundle.Settings.Name, defaultWorkspaceName) + + if len(bundle.Agents) != 1 { + t.Fatalf("agents: got %d, want 1", len(bundle.Agents)) + } + agent := bundle.Agents[0] + assertEqual(t, "agent name", agent.Name, "ada") + assertEqual(t, "agent role", agent.Role, "cto") + assertEqual(t, "agent icon", agent.Icon, "🤖") + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", agent.MaxConcurrentSessions, 3) + assertEqual(t, "agent desired skills", agent.DesiredSkills, `["go","sql"]`) + assertEqual(t, "agent executor preference", agent.ExecutorPreference, "local_docker") + + if len(bundle.Skills) != 1 { + t.Fatalf("skills: got %d, want 1", len(bundle.Skills)) + } + // The loader derives name and slug from the directory and stamps + // source_type "filesystem"; only the body comes from SKILL.md. + assertEqual(t, "skill name", bundle.Skills[0].Name, "code-review") + assertEqual(t, "skill slug", bundle.Skills[0].Slug, "code-review") + assertEqual(t, "skill source type", bundle.Skills[0].SourceType, "filesystem") + assertEqual(t, "skill content", bundle.Skills[0].Content, "# Code Review\n\nread the diff\n") + + if len(bundle.Routines) != 1 { + t.Fatalf("routines: got %d, want 1", len(bundle.Routines)) + } + assertEqual(t, "routine name", bundle.Routines[0].Name, "standup") + assertEqual(t, "routine description", bundle.Routines[0].Description, "daily standup") + assertEqual(t, "routine task template", bundle.Routines[0].TaskTemplate, "summarise yesterday") + assertEqual(t, "routine concurrency", bundle.Routines[0].ConcurrencyPolicy, "skip") + + if len(bundle.Projects) != 1 { + t.Fatalf("projects: got %d, want 1", len(bundle.Projects)) + } + assertEqual(t, "project name", bundle.Projects[0].Name, "apollo") + assertEqual(t, "project description", bundle.Projects[0].Description, "moon shot") + assertEqual(t, "project status", bundle.Projects[0].Status, "paused") + assertEqual(t, "project color", bundle.Projects[0].Color, "#ff00ff") + assertEqual(t, "project budget", bundle.Projects[0].BudgetCents, 99000) + assertEqual(t, "project repositories", bundle.Projects[0].Repositories, `["kdlbs/kandev"]`) + assertEqual(t, "project executor config", bundle.Projects[0].ExecutorConfig, + `{"type":"local_docker"}`) +} + +// TestScanFilesystem_DropsNameReferences records the same gap the importer +// has: the scan does not carry reports_to, assignee_name or lead_agent_name +// off disk even though all three are present in the fixture files. The result +// is a lossy sync — applying a checked-in workspace config erases the org +// chart, routine assignees and project leads rather than resolving the +// portable names to IDs. +// +// Recorded rather than fixed (test-only change); carrying the names through +// the scan and resolving them on import must update this test. +func TestScanFilesystem_DropsNameReferences(t *testing.T) { + env := newTestEnv(t) + seedFullFSState(t, env) + + bundle, _, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + assertEqual(t, "agent reports_to", bundle.Agents[0].ReportsTo, "") + assertEqual(t, "routine assignee", bundle.Routines[0].AssigneeName, "") + assertEqual(t, "project lead", bundle.Projects[0].LeadAgentName, "") +} + +// TestScanFilesystem_NoConfigOnDisk documents that a missing workspaces +// directory is not an error and yields an empty (non-nil) bundle. +func TestScanFilesystem_NoConfigOnDisk(t *testing.T) { + env := newTestEnv(t) + if err := os.RemoveAll(filepath.Join(env.base, "workspaces")); err != nil { + t.Fatalf("remove workspaces dir: %v", err) + } + + bundle, parseErrs, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + if bundle == nil { + t.Fatal("bundle should be non-nil even with no config on disk") + } + assertEqual(t, "settings name", bundle.Settings.Name, defaultWorkspaceName) + assertEqual(t, "agents", len(bundle.Agents), 0) + assertEqual(t, "parse errors", len(parseErrs), 0) +} + +// TestScanFilesystem_ReportsParseErrors covers the malformed-input path: a +// broken file is reported rather than swallowed, and the good files still load. +func TestScanFilesystem_ReportsParseErrors(t *testing.T) { + env := newTestEnv(t) + env.writeFSAgent(t, "ada", fullAgentYAML) + env.writeFSAgent(t, "broken", "name: [unclosed\n") + + bundle, parseErrs, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + if len(parseErrs) != 1 { + t.Fatalf("parse errors: got %d (%+v), want 1", len(parseErrs), parseErrs) + } + assertEqual(t, "parse error workspace", parseErrs[0].WorkspaceID, defaultWorkspaceName) + if !strings.HasSuffix(parseErrs[0].FilePath, "broken.yml") { + t.Errorf("parse error file path %q does not name broken.yml", parseErrs[0].FilePath) + } + if parseErrs[0].Error == "" { + t.Error("parse error message is empty") + } + assertStrings(t, "good agent still loaded", agentBundleNames(bundle.Agents), []string{"ada"}) +} + +// TestScanFilesystem_UnreadableWorkspacesDirIsAnError separates "no config on +// disk" (fine) from "config root is unusable" (an error the caller must see). +func TestScanFilesystem_UnreadableWorkspacesDirIsAnError(t *testing.T) { + env := newTestEnv(t) + wsRoot := filepath.Join(env.base, "workspaces") + if err := os.RemoveAll(wsRoot); err != nil { + t.Fatalf("remove workspaces dir: %v", err) + } + // A regular file where the directory should be: ReadDir fails with + // something other than "not exist". + writeFile(t, wsRoot, "not a directory\n") + + bundle, _, err := env.svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error for an unreadable workspaces dir, got nil") + } + if !strings.HasPrefix(err.Error(), "scan workspaces:") { + t.Errorf("error %q does not start with %q", err.Error(), "scan workspaces:") + } + if bundle != nil { + t.Errorf("bundle should be nil on error, got %+v", bundle) + } +} + +// TestScanFilesystem_NilLoaderIsRejected covers the guard in front of every +// filesystem read. +func TestScanFilesystem_NilLoaderIsRejected(t *testing.T) { + env := newTestEnv(t) + svc := NewConfigService(env.repo, nil, env.writer, logger.Default(), env.activity) + + bundle, parseErrs, err := svc.ScanFilesystem(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error for a nil config loader, got nil") + } + assertEqual(t, "error message", err.Error(), "config loader not initialized") + if bundle != nil || parseErrs != nil { + t.Errorf("bundle and errors should be nil, got %+v / %+v", bundle, parseErrs) + } +} + +func TestParseErrorsFromLoader_EmptyIsNil(t *testing.T) { + env := newTestEnv(t) + if got := parseErrorsFromLoader(env.loader); got != nil { + t.Errorf("expected nil for a clean loader, got %+v", got) + } +} + +// TestIncomingDiff_CombinesPreviewAndDeletions is the FS→DB direction: files +// not yet in the DB are creates, matching names are updates, and DB rows with +// no file are deletions. +func TestIncomingDiff_CombinesPreviewAndDeletions(t *testing.T) { + env := newTestEnv(t) + seedFullFSState(t, env) + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, testWorkspaceID, "gone-agent") + seedSkill(t, env, testWorkspaceID, "gone-skill") + seedRoutine(t, env, testWorkspaceID, "gone-routine") + seedProject(t, env, testWorkspaceID, "gone-project") + + diff, err := env.svc.IncomingDiff(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("IncomingDiff: %v", err) + } + + assertEqual(t, "direction", diff.Direction, "incoming") + if diff.Preview == nil { + t.Fatal("preview is nil") + } + assertEqual(t, "errors", len(diff.Errors), 0) + + assertStrings(t, "agents updated", diff.Preview.Agents.Updated, []string{"ada"}) + assertStrings(t, "agents deleted", diff.Preview.Agents.Deleted, []string{"gone-agent"}) + assertStrings(t, "skills created", diff.Preview.Skills.Created, []string{"code-review"}) + assertStrings(t, "skills deleted", diff.Preview.Skills.Deleted, []string{"gone-skill"}) + assertStrings(t, "routines created", diff.Preview.Routines.Created, []string{"standup"}) + assertStrings(t, "routines deleted", diff.Preview.Routines.Deleted, []string{"gone-routine"}) + assertStrings(t, "projects created", diff.Preview.Projects.Created, []string{"apollo"}) + assertStrings(t, "projects deleted", diff.Preview.Projects.Deleted, []string{"gone-project"}) +} + +func TestIncomingDiff_SurfacesParseErrors(t *testing.T) { + env := newTestEnv(t) + env.writeFSAgent(t, "broken", "name: [unclosed\n") + + diff, err := env.svc.IncomingDiff(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("IncomingDiff: %v", err) + } + if len(diff.Errors) != 1 { + t.Fatalf("errors: got %d (%+v), want 1", len(diff.Errors), diff.Errors) + } +} + +func TestIncomingDiff_PropagatesErrors(t *testing.T) { + t.Run("scan failure", func(t *testing.T) { + env := newTestEnv(t) + svc := NewConfigService(env.repo, nil, env.writer, logger.Default(), env.activity) + if _, err := svc.IncomingDiff(context.Background(), testWorkspaceID); err == nil { + t.Fatal("expected an error for a nil config loader, got nil") + } + }) + t.Run("preview failure", func(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + diff, err := env.svc.IncomingDiff(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error from a closed database, got nil") + } + if diff != nil { + t.Errorf("diff should be nil on error, got %+v", diff) + } + }) + // PreviewImport and appendDeletions read the same four tables, so a + // broken table always surfaces from the preview first. Each entity is + // still worth covering: none of them may be swallowed. + for _, table := range []string{ + "agent_profiles", "office_skills", "office_routines", "office_projects", + } { + t.Run("broken "+table, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, table) + if _, err := env.svc.IncomingDiff(context.Background(), testWorkspaceID); err == nil { + t.Fatalf("expected an error after dropping %s, got nil", table) + } + }) + } +} + +// TestOutgoingDiff_ComparesDatabaseAgainstDisk is the DB→FS direction: DB rows +// with no file are creates, and files with no DB row are deletions. +func TestOutgoingDiff_ComparesDatabaseAgainstDisk(t *testing.T) { + env := newTestEnv(t) + env.writeFSAgent(t, "ada", fullAgentYAML) + env.writeFSAgent(t, "stale", "name: stale\nrole: engineer\n") + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, testWorkspaceID, "fresh") + + diff, err := env.svc.OutgoingDiff(context.Background(), testWorkspaceID) + if err != nil { + t.Fatalf("OutgoingDiff: %v", err) + } + + assertEqual(t, "direction", diff.Direction, "outgoing") + assertEqual(t, "errors", len(diff.Errors), 0) + assertStrings(t, "agents updated", diff.Preview.Agents.Updated, []string{"ada"}) + assertStrings(t, "agents created", diff.Preview.Agents.Created, []string{"fresh"}) + assertStrings(t, "agents deleted", diff.Preview.Agents.Deleted, []string{"stale"}) +} + +func TestOutgoingDiff_PropagatesErrors(t *testing.T) { + t.Run("export failure", func(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + diff, err := env.svc.OutgoingDiff(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error from a closed database, got nil") + } + if diff != nil { + t.Errorf("diff should be nil on error, got %+v", diff) + } + }) + t.Run("scan failure", func(t *testing.T) { + env := newTestEnv(t) + svc := NewConfigService(env.repo, nil, env.writer, logger.Default(), env.activity) + if _, err := svc.OutgoingDiff(context.Background(), testWorkspaceID); err == nil { + t.Fatal("expected an error for a nil config loader, got nil") + } + }) +} + +// TestApplyIncoming_ImportsAndPrunes asserts the full FS→DB apply: files +// create or update rows, and rows with no file are removed. +func TestApplyIncoming_ImportsAndPrunes(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedFullFSState(t, env) + seedAgent(t, env, testWorkspaceID, "ada") + seedAgent(t, env, testWorkspaceID, "gone-agent") + seedSkill(t, env, testWorkspaceID, "gone-skill") + + result, err := env.svc.ApplyIncoming(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ApplyIncoming: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 3) // skill + routine + project + assertEqual(t, "updated count", result.UpdatedCount, 1) // ada + + assertRemainingNames(t, env, testWorkspaceID, []string{"ada"}, []string{"code-review"}, + []string{"standup"}, []string{"apollo"}) + + // The updated agent carries the on-disk values. + agent := agentByName(t, env, testWorkspaceID, "ada") + assertEqual(t, "agent role", string(agent.Role), "cto") + assertEqual(t, "agent budget", agent.BudgetMonthlyCents, 4200) + assertEqual(t, "agent executor preference", agent.ExecutorPreference, "local_docker") +} + +// TestApplyIncoming_IsIdempotent runs the same FS state through twice: the +// second pass updates in place and prunes nothing further. +func TestApplyIncoming_IsIdempotent(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedFullFSState(t, env) + + first, err := env.svc.ApplyIncoming(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("first ApplyIncoming: %v", err) + } + agentID := agentByName(t, env, testWorkspaceID, "ada").ID + + second, err := env.svc.ApplyIncoming(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("second ApplyIncoming: %v", err) + } + assertEqual(t, "first created", first.CreatedCount, 4) + assertEqual(t, "second created", second.CreatedCount, 0) + assertEqual(t, "second updated", second.UpdatedCount, 4) + assertEqual(t, "agent id stable", agentByName(t, env, testWorkspaceID, "ada").ID, agentID) + assertRemainingNames(t, env, testWorkspaceID, []string{"ada"}, []string{"code-review"}, + []string{"standup"}, []string{"apollo"}) +} + +// TestApplyIncoming_EmptyFilesystemClearsWorkspace is the destructive case: +// with no config on disk, the sync prunes every row. +func TestApplyIncoming_EmptyFilesystemClearsWorkspace(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedAgent(t, env, testWorkspaceID, "ada") + seedSkill(t, env, testWorkspaceID, "code-review") + seedRoutine(t, env, testWorkspaceID, "standup") + seedProject(t, env, testWorkspaceID, "apollo") + + result, err := env.svc.ApplyIncoming(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ApplyIncoming: %v", err) + } + assertEqual(t, "created count", result.CreatedCount, 0) + assertEqual(t, "updated count", result.UpdatedCount, 0) + assertRemainingNames(t, env, testWorkspaceID, nil, nil, nil, nil) +} + +func TestApplyIncoming_PropagatesErrors(t *testing.T) { + t.Run("scan failure", func(t *testing.T) { + env := newTestEnv(t) + svc := NewConfigService(env.repo, nil, env.writer, logger.Default(), env.activity) + result, err := svc.ApplyIncoming(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error for a nil config loader, got nil") + } + if result != nil { + t.Errorf("result should be nil on error, got %+v", result) + } + }) + t.Run("import failure", func(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + if _, err := env.svc.ApplyIncoming(context.Background(), testWorkspaceID); err == nil { + t.Fatal("expected an error from a closed database, got nil") + } + }) +} + +// TestApplyOutgoing_WritesDatabaseStateToDisk is the DB→FS apply: every row +// becomes a file the loader can read back. +func TestApplyOutgoing_WritesDatabaseStateToDisk(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedFullDBState(t, env, testWorkspaceID) + + if err := env.svc.ApplyOutgoing(ctx, testWorkspaceID); err != nil { + t.Fatalf("ApplyOutgoing: %v", err) + } + + bundle, _, err := env.svc.ScanFilesystem(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + assertStrings(t, "agents on disk", + sortedCopy(agentBundleNames(bundle.Agents)), []string{"ada", "grace"}) + assertStrings(t, "skills on disk", skillBundleSlugs(bundle.Skills), []string{"code-review"}) + assertStrings(t, "routines on disk", routineBundleNames(bundle.Routines), []string{"standup"}) + assertStrings(t, "projects on disk", projectBundleNames(bundle.Projects), []string{"apollo"}) + + var ada AgentConfig + for _, a := range bundle.Agents { + if a.Name == "ada" { + ada = a + } + } + assertEqual(t, "agent role", ada.Role, "cto") + assertEqual(t, "agent budget", ada.BudgetMonthlyCents, 4200) + assertEqual(t, "agent desired skills", ada.DesiredSkills, `["go","sql"]`) + assertEqual(t, "project status", bundle.Projects[0].Status, "paused") + assertEqual(t, "project budget", bundle.Projects[0].BudgetCents, 99000) +} + +// TestApplyOutgoing_DoesNotDeleteStaleFiles records the asymmetry with +// ApplyIncoming: the incoming apply prunes DB rows missing from disk, but the +// outgoing apply only writes, so a file with no DB row survives even though +// OutgoingDiff just listed it as a deletion. A user who applies the outgoing +// diff in the Sync UI can still commit or later re-import the stale file. +// +// Recorded rather than fixed: making export-fs prune is a production change, +// and it must invert this test. +func TestApplyOutgoing_DoesNotDeleteStaleFiles(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + env.writeFSAgent(t, "stale", "name: stale\nrole: engineer\n") + seedAgent(t, env, testWorkspaceID, "ada") + + diff, err := env.svc.OutgoingDiff(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("OutgoingDiff: %v", err) + } + assertStrings(t, "diff proposes a deletion", diff.Preview.Agents.Deleted, []string{"stale"}) + + if err := env.svc.ApplyOutgoing(ctx, testWorkspaceID); err != nil { + t.Fatalf("ApplyOutgoing: %v", err) + } + if _, err := os.Stat(env.wsPath("agents", "stale.yml")); err != nil { + t.Errorf("stale file should survive an outgoing apply: %v", err) + } +} + +func TestApplyOutgoing_NilWriterIsRejected(t *testing.T) { + env := newTestEnv(t) + svc := NewConfigService(env.repo, env.loader, nil, logger.Default(), env.activity) + + err := svc.ApplyOutgoing(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected an error for a nil config writer, got nil") + } + assertEqual(t, "error message", err.Error(), "config writer not initialized") +} + +func TestApplyOutgoing_PropagatesExportError(t *testing.T) { + env := newTestEnv(t) + env.closeDB(t) + + if err := env.svc.ApplyOutgoing(context.Background(), testWorkspaceID); err == nil { + t.Fatal("expected an error from a closed database, got nil") + } +} + +// TestSyncRoundTrip_DiskToDatabaseToDisk applies incoming then outgoing and +// asserts the values survive both crossings unchanged. +func TestSyncRoundTrip_DiskToDatabaseToDisk(t *testing.T) { + env := newTestEnv(t) + ctx := context.Background() + seedFullFSState(t, env) + + if _, err := env.svc.ApplyIncoming(ctx, testWorkspaceID); err != nil { + t.Fatalf("ApplyIncoming: %v", err) + } + if err := env.svc.ApplyOutgoing(ctx, testWorkspaceID); err != nil { + t.Fatalf("ApplyOutgoing: %v", err) + } + + bundle, _, err := env.svc.ScanFilesystem(ctx, testWorkspaceID) + if err != nil { + t.Fatalf("ScanFilesystem: %v", err) + } + if len(bundle.Agents) != 1 { + t.Fatalf("agents after round trip: got %d, want 1", len(bundle.Agents)) + } + assertEqual(t, "agent name", bundle.Agents[0].Name, "ada") + assertEqual(t, "agent role", bundle.Agents[0].Role, "cto") + assertEqual(t, "agent icon", bundle.Agents[0].Icon, "🤖") + assertEqual(t, "agent budget", bundle.Agents[0].BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", bundle.Agents[0].MaxConcurrentSessions, 3) + assertEqual(t, "agent desired skills", bundle.Agents[0].DesiredSkills, `["go","sql"]`) + assertEqual(t, "agent executor preference", bundle.Agents[0].ExecutorPreference, "local_docker") + + assertEqual(t, "routine description", bundle.Routines[0].Description, "daily standup") + assertEqual(t, "routine task template", bundle.Routines[0].TaskTemplate, "summarise yesterday") + assertEqual(t, "routine concurrency", bundle.Routines[0].ConcurrencyPolicy, "skip") + + assertEqual(t, "project description", bundle.Projects[0].Description, "moon shot") + assertEqual(t, "project color", bundle.Projects[0].Color, "#ff00ff") + assertEqual(t, "project budget", bundle.Projects[0].BudgetCents, 99000) + assertEqual(t, "project repositories", bundle.Projects[0].Repositories, `["kdlbs/kandev"]`) + assertEqual(t, "project executor config", bundle.Projects[0].ExecutorConfig, + `{"type":"local_docker"}`) + + // The project's status is the one value the round trip does not + // preserve: the importer forces every created project to "active". + assertEqual(t, "project status", bundle.Projects[0].Status, "active") +} diff --git a/apps/backend/internal/office/config/sync_util_test.go b/apps/backend/internal/office/config/sync_util_test.go new file mode 100644 index 0000000000..d7ad4e5db8 --- /dev/null +++ b/apps/backend/internal/office/config/sync_util_test.go @@ -0,0 +1,325 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/kandev/kandev/internal/office/configloader" + "github.com/kandev/kandev/internal/office/models" +) + +func TestMissingNames(t *testing.T) { + tests := []struct { + name string + existing []string + incoming []string + want []string + }{ + { + name: "all present leaves nothing missing", + existing: []string{"a", "b"}, + incoming: []string{"b", "a"}, + want: nil, + }, + { + name: "missing entries preserve existing order", + existing: []string{"a", "b", "c"}, + incoming: []string{"b"}, + want: []string{"a", "c"}, + }, + { + name: "empty incoming means everything is missing", + existing: []string{"a", "b"}, + incoming: nil, + want: []string{"a", "b"}, + }, + { + name: "empty existing is never missing", + existing: nil, + incoming: []string{"a"}, + want: nil, + }, + { + name: "extra incoming names are not reported", + existing: []string{"a"}, + incoming: []string{"a", "b", "c"}, + want: nil, + }, + { + name: "duplicate existing names each report", + existing: []string{"a", "a"}, + incoming: []string{"b"}, + want: []string{"a", "a"}, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + assertStrings(t, "missingNames", missingNames(tc.existing, tc.incoming), tc.want) + }) + } +} + +func TestModelNameExtractors(t *testing.T) { + agents := []*models.AgentInstance{{Name: "ada"}, {Name: "grace"}} + assertStrings(t, "agentNames", agentNames(agents), []string{"ada", "grace"}) + assertStrings(t, "agentNames empty", agentNames(nil), []string{}) + + skills := []*models.Skill{{Slug: "code-review"}, {Slug: "triage"}} + assertStrings(t, "skillSlugs", skillSlugs(skills), []string{"code-review", "triage"}) + assertStrings(t, "skillSlugs empty", skillSlugs(nil), []string{}) + + routines := []*models.Routine{{Name: "standup"}} + assertStrings(t, "routineNames", routineNames(routines), []string{"standup"}) + assertStrings(t, "routineNames empty", routineNames(nil), []string{}) + + projects := []*models.Project{{Name: "apollo"}, {Name: "gemini"}} + assertStrings(t, "projectNames", projectNames(projects), []string{"apollo", "gemini"}) + assertStrings(t, "projectNames empty", projectNames(nil), []string{}) +} + +func TestBundleNameExtractors(t *testing.T) { + bundle := fullBundle() + bundle.Agents = append(bundle.Agents, AgentConfig{Name: "grace"}) + bundle.Skills = append(bundle.Skills, SkillConfig{Slug: "triage"}) + bundle.Routines = append(bundle.Routines, RoutineConfig{Name: "retro"}) + bundle.Projects = append(bundle.Projects, ProjectConfig{Name: "gemini"}) + + assertStrings(t, "agentBundleNames", agentBundleNames(bundle.Agents), []string{"ada", "grace"}) + assertStrings(t, "skillBundleSlugs", skillBundleSlugs(bundle.Skills), []string{"code-review", "triage"}) + assertStrings(t, "routineBundleNames", routineBundleNames(bundle.Routines), []string{"standup", "retro"}) + assertStrings(t, "projectBundleNames", projectBundleNames(bundle.Projects), []string{"apollo", "gemini"}) +} + +func TestBundleSets(t *testing.T) { + bundle := fullBundle() + + // Agents key on Name, skills on Slug — a set keyed on the wrong field + // would make every diff report spurious creations. + agentSet := bundleAgentSet(bundle.Agents) + assertEqual(t, "agent set size", len(agentSet), 1) + assertEqual(t, "agent set has ada", agentSet["ada"], true) + assertEqual(t, "agent set lacks cto role", agentSet["cto"], false) + + skillSet := bundleSkillSet(bundle.Skills) + assertEqual(t, "skill set has slug", skillSet["code-review"], true) + assertEqual(t, "skill set does not key on name", skillSet["Code Review"], false) + + routineSet := bundleRoutineSet(bundle.Routines) + assertEqual(t, "routine set has standup", routineSet["standup"], true) + + projectSet := bundleProjectSet(bundle.Projects) + assertEqual(t, "project set has apollo", projectSet["apollo"], true) + + assertEqual(t, "empty agent set", len(bundleAgentSet(nil)), 0) + assertEqual(t, "empty skill set", len(bundleSkillSet(nil)), 0) + assertEqual(t, "empty routine set", len(bundleRoutineSet(nil)), 0) + assertEqual(t, "empty project set", len(bundleProjectSet(nil)), 0) +} + +// TestWriteBundleToFS_AllFieldsReachDisk writes a fully-populated bundle and +// reads it back through the ConfigLoader. Every field that survives the +// YAML round trip is asserted; a field dropped by writeBundleEntities would +// come back zero-valued instead of failing loudly at write time. +func TestWriteBundleToFS_AllFieldsReachDisk(t *testing.T) { + env := newTestEnv(t) + bundle := fullBundle() + + if err := writeBundleToFS(env.writer, bundle); err != nil { + t.Fatalf("writeBundleToFS: %v", err) + } + + // The writer always targets the "default" FS workspace, not the + // bundle's settings name. + if _, err := os.Stat(env.wsPath("agents", "ada.yml")); err != nil { + t.Fatalf("agent file not written to default workspace: %v", err) + } + + agents := env.loader.GetAgents(defaultWorkspaceName) + if len(agents) != 1 { + t.Fatalf("agents on disk: got %d, want 1", len(agents)) + } + got := agents[0] + assertEqual(t, "agent name", got.Name, "ada") + assertEqual(t, "agent role", string(got.Role), "cto") + assertEqual(t, "agent icon", got.Icon, "🤖") + assertEqual(t, "agent budget", got.BudgetMonthlyCents, 4200) + assertEqual(t, "agent max sessions", got.MaxConcurrentSessions, 3) + assertEqual(t, "agent desired skills", got.DesiredSkills, `["go","sql"]`) + assertEqual(t, "agent executor preference", got.ExecutorPreference, "local_docker") + + skills := env.loader.GetSkills(defaultWorkspaceName) + if len(skills) != 1 { + t.Fatalf("skills on disk: got %d, want 1", len(skills)) + } + assertEqual(t, "skill slug", skills[0].Slug, "code-review") + assertEqual(t, "skill content", skills[0].Content, "# Code Review\n\nread the diff\n") + + routines := env.loader.GetRoutines(defaultWorkspaceName) + if len(routines) != 1 { + t.Fatalf("routines on disk: got %d, want 1", len(routines)) + } + assertEqual(t, "routine name", routines[0].Name, "standup") + assertEqual(t, "routine description", routines[0].Description, "daily standup") + assertEqual(t, "routine task template", routines[0].TaskTemplate, "summarise yesterday") + assertEqual(t, "routine concurrency", string(routines[0].ConcurrencyPolicy), "skip") + + projects := env.loader.GetProjects(defaultWorkspaceName) + if len(projects) != 1 { + t.Fatalf("projects on disk: got %d, want 1", len(projects)) + } + assertEqual(t, "project name", projects[0].Name, "apollo") + assertEqual(t, "project description", projects[0].Description, "moon shot") + assertEqual(t, "project status", string(projects[0].Status), "paused") + assertEqual(t, "project color", projects[0].Color, "#ff00ff") + assertEqual(t, "project budget", projects[0].BudgetCents, 99000) + assertEqual(t, "project repositories", projects[0].Repositories, `["kdlbs/kandev"]`) + assertEqual(t, "project executor config", projects[0].ExecutorConfig, `{"type":"local_docker"}`) +} + +// TestWriteBundleToFS_EmptySkillContentGetsHeading covers the one value +// writeBundleEntities synthesises rather than copies. +func TestWriteBundleToFS_EmptySkillContentGetsHeading(t *testing.T) { + env := newTestEnv(t) + bundle := &ConfigBundle{Skills: []SkillConfig{{Name: "Triage Bugs", Slug: "triage"}}} + + if err := writeBundleToFS(env.writer, bundle); err != nil { + t.Fatalf("writeBundleToFS: %v", err) + } + + data, err := os.ReadFile(env.wsPath("skills", "triage", "SKILL.md")) + if err != nil { + t.Fatalf("read SKILL.md: %v", err) + } + assertEqual(t, "synthesised skill content", string(data), "# Triage Bugs\n") +} + +func TestWriteBundleToFS_EmptyBundleWritesNothing(t *testing.T) { + env := newTestEnv(t) + + if err := writeBundleToFS(env.writer, &ConfigBundle{}); err != nil { + t.Fatalf("writeBundleToFS: %v", err) + } + + for _, dir := range []string{"agents", "skills", "routines", "projects"} { + if _, err := os.Stat(env.wsPath(dir)); !os.IsNotExist(err) { + t.Errorf("%s dir should not exist for an empty bundle (err=%v)", dir, err) + } + } +} + +// TestWriteBundleToFS_RejectsUnsafeNames pins the FileWriter's path-component +// validation reaching back through writeBundleEntities: a traversal name must +// abort the write rather than escape the workspace directory. +func TestWriteBundleToFS_RejectsUnsafeNames(t *testing.T) { + // escapedPath is where the write would land if the name were joined + // unchecked: "../escape" climbs out of the per-entity subdirectory into + // the workspace root. Asserting on that exact path matters — an + // arbitrary non-existent path would pass no matter what the writer did. + tests := []struct { + name string + bundle *ConfigBundle + escapedPath []string + }{ + { + "agent traversal", + &ConfigBundle{Agents: []AgentConfig{{Name: "../escape"}}}, + []string{"escape.yml"}, + }, + { + "skill traversal", + &ConfigBundle{Skills: []SkillConfig{{Slug: "../escape"}}}, + []string{"escape", "SKILL.md"}, + }, + { + "routine traversal", + &ConfigBundle{Routines: []RoutineConfig{{Name: "../escape"}}}, + []string{"escape.yml"}, + }, + { + "project traversal", + &ConfigBundle{Projects: []ProjectConfig{{Name: "../escape"}}}, + []string{"escape.yml"}, + }, + { + "empty agent name", + &ConfigBundle{Agents: []AgentConfig{{Name: ""}}}, + nil, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newTestEnv(t) + err := writeBundleToFS(env.writer, tc.bundle) + if err == nil { + t.Fatal("expected an error for an unsafe name, got nil") + } + if tc.escapedPath == nil { + return + } + escaped := env.wsPath(tc.escapedPath...) + if _, statErr := os.Stat(escaped); !os.IsNotExist(statErr) { + t.Errorf("write escaped into %s: %v", escaped, statErr) + } + }) + } +} + +// TestWriteBundleToFS_StopsAtFirstError proves the write is not best-effort: +// the bad entity aborts the loop, so the entity after it is never written. +func TestWriteBundleToFS_StopsAtFirstError(t *testing.T) { + env := newTestEnv(t) + bundle := &ConfigBundle{Agents: []AgentConfig{ + {Name: "ada"}, + {Name: "../escape"}, + {Name: "grace"}, + }} + + if err := writeBundleToFS(env.writer, bundle); err == nil { + t.Fatal("expected an error, got nil") + } + if _, err := os.Stat(env.wsPath("agents", "ada.yml")); err != nil { + t.Errorf("agent before the failure should have been written: %v", err) + } + if _, err := os.Stat(env.wsPath("agents", "grace.yml")); !os.IsNotExist(err) { + t.Errorf("agent after the failure should not have been written: %v", err) + } +} + +// TestWriteBundleToFS_UsesDefaultWorkspaceOnly documents that the on-disk +// target is always the "default" workspace directory: the bundle's settings +// name does not steer the writer. +func TestWriteBundleToFS_UsesDefaultWorkspaceOnly(t *testing.T) { + env := newTestEnv(t) + bundle := &ConfigBundle{ + Settings: SettingsConfig{Name: "some-other-workspace"}, + Agents: []AgentConfig{{Name: "ada", Role: "cto"}}, + } + + if err := writeBundleToFS(env.writer, bundle); err != nil { + t.Fatalf("writeBundleToFS: %v", err) + } + + if _, err := os.Stat(env.wsPath("agents", "ada.yml")); err != nil { + t.Errorf("agent not written under %q: %v", defaultWorkspaceName, err) + } + other := filepath.Join(env.base, "workspaces", "some-other-workspace") + if _, err := os.Stat(other); !os.IsNotExist(err) { + t.Errorf("settings name should not create a workspace dir: %v", err) + } +} + +// TestWriteBundleEntities_EmptyIsNoOp guards the direct helper entry point used +// by ApplyOutgoing after its nil check: an empty bundle creates no directories. +func TestWriteBundleEntities_EmptyIsNoOp(t *testing.T) { + base := t.TempDir() + loader := configloader.NewConfigLoader(base) + writer := configloader.NewFileWriter(base, loader) + + if err := writeBundleEntities(writer, &ConfigBundle{}); err != nil { + t.Fatalf("writeBundleEntities: %v", err) + } + if _, err := os.Stat(filepath.Join(base, "workspaces")); !os.IsNotExist(err) { + t.Errorf("no workspace dir should be created: %v", err) + } +} diff --git a/apps/backend/internal/office/config/write_errors_test.go b/apps/backend/internal/office/config/write_errors_test.go new file mode 100644 index 0000000000..0917682657 --- /dev/null +++ b/apps/backend/internal/office/config/write_errors_test.go @@ -0,0 +1,172 @@ +package config + +import ( + "context" + "strings" + "testing" +) + +// TestApplyImport_PropagatesCreateErrors breaks the write handle after the +// tables are readable, so every List succeeds and every Create fails. Each +// entity's create branch must surface the error instead of counting a row it +// never wrote. +func TestApplyImport_PropagatesCreateErrors(t *testing.T) { + tests := []struct { + name string + bundle *ConfigBundle + wantPrefix string + }{ + {"agents", &ConfigBundle{Agents: []AgentConfig{{Name: "ada"}}}, "apply agents:"}, + {"skills", &ConfigBundle{Skills: []SkillConfig{{Slug: "code-review"}}}, "apply skills:"}, + {"routines", &ConfigBundle{Routines: []RoutineConfig{{Name: "standup"}}}, "apply routines:"}, + {"projects", &ConfigBundle{Projects: []ProjectConfig{{Name: "apollo"}}}, "apply projects:"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newSplitTestEnv(t) + env.breakWrites(t) + + result, err := env.svc.ApplyImport(context.Background(), testWorkspaceID, tc.bundle) + if err == nil { + t.Fatal("expected a write error, got nil") + } + if result != nil { + t.Errorf("result should be nil on error, got %+v", result) + } + if !strings.HasPrefix(err.Error(), tc.wantPrefix) { + t.Errorf("error %q does not start with %q", err.Error(), tc.wantPrefix) + } + }) + } +} + +// TestApplyImport_PropagatesUpdateErrors is the same guard for the update +// branch: the rows already exist, so each stage takes the update path. +func TestApplyImport_PropagatesUpdateErrors(t *testing.T) { + tests := []struct { + name string + bundle *ConfigBundle + wantPrefix string + }{ + {"agents", &ConfigBundle{Agents: []AgentConfig{{Name: "ada"}}}, "apply agents:"}, + {"skills", &ConfigBundle{Skills: []SkillConfig{{Slug: "code-review"}}}, "apply skills:"}, + {"routines", &ConfigBundle{Routines: []RoutineConfig{{Name: "standup"}}}, "apply routines:"}, + {"projects", &ConfigBundle{Projects: []ProjectConfig{{Name: "apollo"}}}, "apply projects:"}, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newSplitTestEnv(t) + seedAgent(t, env, testWorkspaceID, "ada") + seedSkill(t, env, testWorkspaceID, "code-review") + seedRoutine(t, env, testWorkspaceID, "standup") + seedProject(t, env, testWorkspaceID, "apollo") + env.breakWrites(t) + + _, err := env.svc.ApplyImport(context.Background(), testWorkspaceID, tc.bundle) + if err == nil { + t.Fatal("expected a write error, got nil") + } + if !strings.HasPrefix(err.Error(), tc.wantPrefix) { + t.Errorf("error %q does not start with %q", err.Error(), tc.wantPrefix) + } + }) + } +} + +// TestDeleteRowsMissingFromBundle_PropagatesDeleteErrors covers the prune +// path's write failures, one entity at a time. +// +// deleteRowsMissingFromBundle returns repository errors unwrapped, so the +// error text cannot say which stage failed. The stage is isolated by the +// fixture instead: each subtest seeds only its own entity, so every earlier +// stage finds nothing to delete and returns before attempting a write. The +// surviving-row assertion then confirms the delete was both attempted and +// refused, rather than skipped. +func TestDeleteRowsMissingFromBundle_PropagatesDeleteErrors(t *testing.T) { + tests := []struct { + name string + seed func(t *testing.T, env *testEnv) + remains func(t *testing.T, env *testEnv) + }{ + { + "agents", + func(t *testing.T, env *testEnv) { seedAgent(t, env, testWorkspaceID, "gone") }, + func(t *testing.T, env *testEnv) { + assertRemainingNames(t, env, testWorkspaceID, []string{"gone"}, nil, nil, nil) + }, + }, + { + "skills", + func(t *testing.T, env *testEnv) { seedSkill(t, env, testWorkspaceID, "gone") }, + func(t *testing.T, env *testEnv) { + assertRemainingNames(t, env, testWorkspaceID, nil, []string{"gone"}, nil, nil) + }, + }, + { + "routines", + func(t *testing.T, env *testEnv) { seedRoutine(t, env, testWorkspaceID, "gone") }, + func(t *testing.T, env *testEnv) { + assertRemainingNames(t, env, testWorkspaceID, nil, nil, []string{"gone"}, nil) + }, + }, + { + "projects", + func(t *testing.T, env *testEnv) { seedProject(t, env, testWorkspaceID, "gone") }, + func(t *testing.T, env *testEnv) { + assertRemainingNames(t, env, testWorkspaceID, nil, nil, nil, []string{"gone"}) + }, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + env := newSplitTestEnv(t) + tc.seed(t, env) + env.breakWrites(t) + + err := env.svc.deleteRowsMissingFromBundle( + context.Background(), testWorkspaceID, &ConfigBundle{}) + if err == nil { + t.Fatal("expected a delete error, got nil") + } + tc.remains(t, env) + }) + } +} + +// TestApplyIncoming_PropagatesPruneError proves a failed prune fails the whole +// sync rather than returning a success result for a half-applied state. +func TestApplyIncoming_PropagatesPruneError(t *testing.T) { + env := newSplitTestEnv(t) + seedAgent(t, env, testWorkspaceID, "gone") + env.breakWrites(t) + + result, err := env.svc.ApplyIncoming(context.Background(), testWorkspaceID) + if err == nil { + t.Fatal("expected a prune error, got nil") + } + if result != nil { + t.Errorf("result should be nil on error, got %+v", result) + } +} + +// TestPreviewImport_PropagatesPerEntityErrors drops one table at a time so +// each preview stage fails on its own. +func TestPreviewImport_PropagatesPerEntityErrors(t *testing.T) { + for _, table := range []string{ + "agent_profiles", "office_skills", "office_routines", "office_projects", + } { + t.Run(table, func(t *testing.T) { + env := newTestEnv(t) + env.dropTable(t, table) + + preview, err := env.svc.PreviewImport( + context.Background(), testWorkspaceID, fullBundle()) + if err == nil { + t.Fatalf("expected an error after dropping %s, got nil", table) + } + if preview != nil { + t.Errorf("preview should be nil on error, got %+v", preview) + } + }) + } +}