diff --git a/workspace-server/internal/handlers/org.go b/workspace-server/internal/handlers/org.go index bd8e2567..a25fac99 100644 --- a/workspace-server/internal/handlers/org.go +++ b/workspace-server/internal/handlers/org.go @@ -4,6 +4,7 @@ package handlers // Tree creation logic is in org_import.go; utility helpers in org_helpers.go. import ( + "bufio" "context" "encoding/json" "fmt" @@ -147,6 +148,17 @@ func sizeOfSubtree(ws OrgWorkspace) nodeSize { } } +// childSlot returns the (x, y) position of child `index` in a 2-column +// fixed-size grid. Used as the default when sibling sizes are unknown. +// Formula: x = parentSidePadding + col*(childDefaultWidth+childGutter), +// y = parentHeaderPadding + row*(childDefaultHeight+childGutter). +func childSlot(index int) (x, y float64) { + col := index % childGridColumnCount + row := index / childGridColumnCount + return parentSidePadding + float64(col)*(childDefaultWidth+childGutter), + parentHeaderPadding + float64(row)*(childDefaultHeight+childGutter) +} + // childSlotInGrid computes the relative position of sibling `index` // given all siblings' subtree sizes. Uniform column width (= max width // across siblings), per-row max height, so a nested parent sibling @@ -328,6 +340,95 @@ func (e *EnvRequirement) UnmarshalJSON(data []byte) error { return nil } +// perWorkspaceUnsatisfied is the return type of collectPerWorkspaceUnsatisfied. +// Each entry names the workspace and files_dir that declared an unsatisfied +// requirement, plus the requirement itself (EnvRequirement serialises to +// the same dual shape {string | {any_of: [...]}} in the 412 JSON response). +type perWorkspaceUnsatisfied struct { + Workspace string `json:"workspace"` + FilesDir string `json:"files_dir"` + Unsatisfied EnvRequirement `json:"unsatisfied"` +} + +// collectPerWorkspaceUnsatisfied walks the workspace tree and reports every +// RequiredEnv that is not covered by global secrets (configured) or by an +// on-disk .env file. orgBaseDir is the on-disk root of the org template +// (each workspace's .env lives at orgBaseDir//.env); when empty +// no .env files are checked and only global coverage can satisfy a requirement. +// A workspace is satisfied by the .env in its own files_dir AND the org root +// .env (env vars cascade downward from the root). +func collectPerWorkspaceUnsatisfied( + workspaces []OrgWorkspace, + orgBaseDir string, + configured map[string]struct{}, +) []perWorkspaceUnsatisfied { + var result []perWorkspaceUnsatisfied + for _, ws := range workspaces { + // Check each RequiredEnv. + for _, req := range ws.RequiredEnv { + if req.IsSatisfied(configured) { + continue + } + // Not covered by global secrets — check .env files if available. + // When orgBaseDir is empty (inline template import) we cannot check + // .env files, so any key not in configured is genuinely missing. + if orgBaseDir == "" || !envKeyPresent(orgBaseDir, ws.FilesDir, req.Members()...) { + result = append(result, perWorkspaceUnsatisfied{ + Workspace: ws.Name, + FilesDir: ws.FilesDir, + Unsatisfied: req, + }) + } + } + // Recurse into children so deeply nested workspaces are also checked. + result = append(result, collectPerWorkspaceUnsatisfied(ws.Children, orgBaseDir, configured)...) + } + return result +} + +// envKeyPresent checks whether all env keys appear in either +// orgBaseDir/.env (root) or orgBaseDir/filesDir/.env (workspace). +// Returns true only when all keys are found in at least one of those files. +func envKeyPresent(orgBaseDir, filesDir string, keys ...string) bool { + if len(keys) == 0 { + return true + } + // Load root .env (covers vars that cascade from org root). + rootEnv := loadEnvVars(orgBaseDir + "/.env") + // Load workspace .env. + wsEnv := loadEnvVars(orgBaseDir + "/" + filesDir + "/.env") + for _, k := range keys { + if _, inRoot := rootEnv[k]; !inRoot { + if _, inWS := wsEnv[k]; !inWS { + return false + } + } + } + return true +} + +// loadEnvVars reads a .env file and returns keys→values. +func loadEnvVars(path string) map[string]string { + vars := map[string]string{} + f, err := os.Open(path) + if err != nil { + return vars + } + defer f.Close() + sc := bufio.NewScanner(f) + for sc.Scan() { + line := strings.TrimSpace(sc.Text()) + if line == "" || strings.HasPrefix(line, "#") { + continue + } + parts := strings.SplitN(line, "=", 2) + if len(parts) == 2 { + vars[parts[0]] = parts[1] + } + } + return vars +} + // OrgTemplate is the YAML structure for an org hierarchy. type OrgTemplate struct { Name string `yaml:"name" json:"name"` diff --git a/workspace-server/internal/handlers/org_helpers_pure_test.go b/workspace-server/internal/handlers/org_helpers_pure_test.go index 6e3a8a50..c92584b9 100644 --- a/workspace-server/internal/handlers/org_helpers_pure_test.go +++ b/workspace-server/internal/handlers/org_helpers_pure_test.go @@ -643,7 +643,13 @@ func TestMergePlugins_ExclusionWithBang(t *testing.T) { } func TestMergePlugins_ExclusionWithDash(t *testing.T) { + // Exclusion pattern with trailing dash removes that plugin from defaults. defaults := []string{"plugin-a", "plugin-b", "plugin-c"} + wsPlugins := []string{"!plugin-b"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-c"}, result) +} + func TestMergePlugins_ExclusionEmptyTarget(t *testing.T) { defaults := []string{"plugin-a", "plugin-b"} wsPlugins := []string{"!", "-"} // no-op exclusions @@ -660,7 +666,13 @@ func TestMergePlugins_ExclusionNotInDefaults(t *testing.T) { } func TestMergePlugins_WorkspaceAddsNew(t *testing.T) { + // Workspace can add new plugins not present in defaults. defaults := []string{"plugin-a"} + wsPlugins := []string{"plugin-a", "plugin-b"} + result := mergePlugins(defaults, wsPlugins) + assert.Equal(t, []string{"plugin-a", "plugin-b"}, result) +} + func TestMergePlugins_DeduplicationOrder(t *testing.T) { // Defaults first; workspace entries deduplicated. defaults := []string{"plugin-a", "plugin-a", "plugin-b"} diff --git a/workspace-server/internal/handlers/plugins_atomic_test.go b/workspace-server/internal/handlers/plugins_atomic_test.go index aef0b50c..92cf3c74 100644 --- a/workspace-server/internal/handlers/plugins_atomic_test.go +++ b/workspace-server/internal/handlers/plugins_atomic_test.go @@ -215,9 +215,9 @@ func TestTarWalk_EmptyDirectory(t *testing.T) { } } -// TestTarWalk_NestedDirs: deeply nested directories produce all intermediate +// TestTarWalk_NestedDirs_Atomic: deeply nested directories produce all intermediate // dir entries plus leaf entries. This exercises the recursive walk. -func TestTarWalk_NestedDirs(t *testing.T) { +func TestTarWalk_NestedDirs_Atomic(t *testing.T) { hostDir := t.TempDir() deep := filepath.Join(hostDir, "a", "b", "c") if err := os.MkdirAll(deep, 0o755); err != nil { diff --git a/workspace-server/internal/handlers/workspace_crud_test.go b/workspace-server/internal/handlers/workspace_crud_test.go index 953f67b8..caac97e3 100644 --- a/workspace-server/internal/handlers/workspace_crud_test.go +++ b/workspace-server/internal/handlers/workspace_crud_test.go @@ -38,7 +38,7 @@ func setupWorkspaceCrudTest(t *testing.T) (sqlmock.Sqlmock, *gin.Engine) { func TestState_LegacyWorkspaceNoLiveToken(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + h := NewWorkspaceHandler(nil, nil, "", "") r.GET("/workspaces/:id/state", h.State) wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" @@ -76,7 +76,7 @@ func TestState_LegacyWorkspaceNoLiveToken(t *testing.T) { func TestState_HasLiveTokenMissingAuth(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + h := NewWorkspaceHandler(nil, nil, "", "") r.GET("/workspaces/:id/state", h.State) wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" @@ -96,7 +96,7 @@ func TestState_HasLiveTokenMissingAuth(t *testing.T) { func TestState_WorkspaceNotFound(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + h := NewWorkspaceHandler(nil, nil, "", "") r.GET("/workspaces/:id/state", h.State) wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" @@ -126,7 +126,7 @@ func TestState_WorkspaceNotFound(t *testing.T) { func TestState_WorkspaceSoftDeleted(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + h := NewWorkspaceHandler(nil, nil, "", "") r.GET("/workspaces/:id/state", h.State) wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" @@ -159,7 +159,7 @@ func TestState_WorkspaceSoftDeleted(t *testing.T) { func TestState_QueryError(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + h := NewWorkspaceHandler(nil, nil, "", "") r.GET("/workspaces/:id/state", h.State) wsID := "aaaaaaaa-aaaa-aaaa-aaaa-aaaaaaaaaaaa" @@ -182,8 +182,9 @@ func TestState_QueryError(t *testing.T) { // ---------- Update ---------- func TestUpdate_InvalidUUID(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -200,8 +201,9 @@ func TestUpdate_InvalidUUID(t *testing.T) { } func TestUpdate_InvalidBody(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -217,7 +219,8 @@ func TestUpdate_InvalidBody(t *testing.T) { func TestUpdate_WorkspaceNotFound(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _ = r + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -240,8 +243,9 @@ func TestUpdate_WorkspaceNotFound(t *testing.T) { } func TestUpdate_NameTooLong(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -262,8 +266,9 @@ func TestUpdate_NameTooLong(t *testing.T) { } func TestUpdate_RoleTooLong(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -284,8 +289,9 @@ func TestUpdate_RoleTooLong(t *testing.T) { } func TestUpdate_NameWithNewline(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -302,8 +308,9 @@ func TestUpdate_NameWithNewline(t *testing.T) { } func TestUpdate_NameWithYAMLSpecialChars(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -320,8 +327,9 @@ func TestUpdate_NameWithYAMLSpecialChars(t *testing.T) { } func TestUpdate_WorkspaceDirSystemPath(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -338,8 +346,9 @@ func TestUpdate_WorkspaceDirSystemPath(t *testing.T) { } func TestUpdate_WorkspaceDirTraversal(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -356,8 +365,9 @@ func TestUpdate_WorkspaceDirTraversal(t *testing.T) { } func TestUpdate_WorkspaceDirRelativePath(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.PATCH("/workspaces/:id", h.Update) @@ -376,8 +386,9 @@ func TestUpdate_WorkspaceDirRelativePath(t *testing.T) { // ---------- Delete ---------- func TestDelete_InvalidUUID(t *testing.T) { - _, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _, _unused := setupWorkspaceCrudTest(t) + _ = _unused + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.DELETE("/workspaces/:id", h.Delete) @@ -392,7 +403,8 @@ func TestDelete_InvalidUUID(t *testing.T) { func TestDelete_HasChildrenWithoutConfirm(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _ = r + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.DELETE("/workspaces/:id", h.Delete) @@ -426,7 +438,8 @@ func TestDelete_HasChildrenWithoutConfirm(t *testing.T) { func TestDelete_ChildrenCheckQueryError(t *testing.T) { mock, r := setupWorkspaceCrudTest(t) - h := NewWorkspaceHandler(nil, nil, nil, nil) + _ = r + h := NewWorkspaceHandler(nil, nil, "", "") r2 := gin.New() r2.DELETE("/workspaces/:id", h.Delete) diff --git a/workspace-server/internal/handlers/workspace_crud_validators_test.go b/workspace-server/internal/handlers/workspace_crud_validators_test.go index 1983808d..6bddb291 100644 --- a/workspace-server/internal/handlers/workspace_crud_validators_test.go +++ b/workspace-server/internal/handlers/workspace_crud_validators_test.go @@ -6,7 +6,7 @@ import ( // ── validateWorkspaceID ───────────────────────────────────────────────────────── -func TestValidateWorkspaceID_Valid(t *testing.T) { +func TestValidateWorkspaceID_Validators_Valid(t *testing.T) { cases := []string{ "550e8400-e29b-41d4-a716-446655440000", "00000000-0000-0000-0000-000000000000", @@ -21,7 +21,7 @@ func TestValidateWorkspaceID_Valid(t *testing.T) { } } -func TestValidateWorkspaceID_Invalid(t *testing.T) { +func TestValidateWorkspaceID_Validators_Invalid(t *testing.T) { cases := []struct { name string id string @@ -47,7 +47,7 @@ func TestValidateWorkspaceID_Invalid(t *testing.T) { // ── validateWorkspaceDir ─────────────────────────────────────────────────────── -func TestValidateWorkspaceDir_Valid(t *testing.T) { +func TestValidateWorkspaceDir_Validators_Valid(t *testing.T) { cases := []string{ "/opt/molecule/workspaces/dev", "/home/user/.molecule/workspaces", @@ -150,13 +150,13 @@ func TestValidateWorkspaceFields_AllEmpty(t *testing.T) { } } -func TestValidateWorkspaceFields_Valid(t *testing.T) { +func TestValidateWorkspaceFields_Validators_Valid(t *testing.T) { if err := validateWorkspaceFields("My Workspace", "Backend Engineer", "gpt-4o", "langgraph"); err != nil { t.Errorf("validateWorkspaceFields with valid args: expected nil, got %v", err) } } -func TestValidateWorkspaceFields_NameTooLong(t *testing.T) { +func TestValidateWorkspaceFields_Validators_NameTooLong(t *testing.T) { longName := make([]byte, 256) for i := range longName { longName[i] = 'a' @@ -175,7 +175,7 @@ func TestValidateWorkspaceFields_NameTooLong(t *testing.T) { } } -func TestValidateWorkspaceFields_RoleTooLong(t *testing.T) { +func TestValidateWorkspaceFields_Validators_RoleTooLong(t *testing.T) { longRole := make([]byte, 1001) for i := range longRole { longRole[i] = 'x' @@ -205,7 +205,7 @@ func TestValidateWorkspaceFields_RuntimeTooLong(t *testing.T) { } } -func TestValidateWorkspaceFields_NewlineInName(t *testing.T) { +func TestValidateWorkspaceFields_Validators_NewlineInName(t *testing.T) { if err := validateWorkspaceFields("My\nWorkspace", "", "", ""); err == nil { t.Error("name with \\n: expected error, got nil") }