test(handlers): fix delegation and MCP tests exposed by full platform suite

Commit b9311134 added a CanCommunicate hierarchy check to proxyA2ARequest.
The executeDelegation tests call proxyA2ARequest directly but did not mock
the workspace hierarchy lookup, causing sqlmock to reject the unexpected
queries and fall through to the DELEGATION_FAILED path.

Fix: add mockCanCommunicate(mock, testSourceID, testTargetID, true) to
each affected executeDelegation test so the hierarchy check returns true
and the proxy is actually called.

Separately, TestMCPHandler_CommitMemory_GlobalScope_Blocked expected
zero DB calls (the handler should block GLOBAL scope before any DB work).
With no memv2 wired, toolCommitMemory fell through to the legacy shim,
which called scopeToWritableNamespace before the scope check.

Fix: wire memv2 via withMemoryV2APIs(nil, nil) in newMCPHandler so the
v2 path is taken and the GLOBAL scope check fires before any DB call.

Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

workspace-server/internal/handlers/delegation_test.go

@@ -1078,6 +1078,9 @@ func TestExecuteDelegation_DeliveryConfirmedProxyError_TreatsAsSuccess(t *testin
 	allowLoopbackForTest(t)
 
 	expectExecuteDelegationBase(mock)
+	// CanCommunicate: workspace hierarchy check added in b9311134; must be mocked
+	// so proxyA2ARequest doesn't return 403 and trigger the failure path.
+	mockCanCommunicate(mock, testSourceID, testTargetID, true)
 	expectExecuteDelegationSuccess(mock, `{"result":{"parts":[{"text":"work completed successfully"}]}}`)
 
 	// Execute synchronously (not as a goroutine) so we can check DB state immediately.
@@ -1148,6 +1151,9 @@ func TestExecuteDelegation_ProxyErrorNon2xx_RemainsFailed(t *testing.T) {
 	allowLoopbackForTest(t)
 
 	expectExecuteDelegationBase(mock)
+	// CanCommunicate: workspace hierarchy check added in b9311134; must be mocked
+	// so proxyA2ARequest doesn't return 403 and trigger the failure path.
+	mockCanCommunicate(mock, testSourceID, testTargetID, true)
 	expectExecuteDelegationFailed(mock)
 
 	a2aBody, _ := json.Marshal(map[string]interface{}{
@@ -1195,6 +1201,9 @@ func TestExecuteDelegation_ProxyErrorEmptyBody_RemainsFailed(t *testing.T) {
 
 	// First attempt: updateDelegationStatus(dispatched) — from expectExecuteDelegationBase
 	expectExecuteDelegationBase(mock)
+	// CanCommunicate: workspace hierarchy check added in b9311134; must be mocked
+	// so proxyA2ARequest doesn't return 403 and trigger the failure path.
+	mockCanCommunicate(mock, testSourceID, testTargetID, true)
 	// Second attempt (retry): updateDelegationStatus(dispatched) again
 	mock.ExpectExec("UPDATE activity_logs SET status").
 		WithArgs("dispatched", "", testSourceID, testDelegationID).
@@ -1243,6 +1252,9 @@ func TestExecuteDelegation_CleanProxyResponse_Unchanged(t *testing.T) {
 	allowLoopbackForTest(t)
 
 	expectExecuteDelegationBase(mock)
+	// CanCommunicate: workspace hierarchy check added in b9311134; must be mocked
+	// so proxyA2ARequest doesn't return 403 and trigger the failure path.
+	mockCanCommunicate(mock, testSourceID, testTargetID, true)
 	expectExecuteDelegationSuccess(mock, `{"result":{"parts":[{"text":"all good"}]}}`)
 
 	a2aBody, _ := json.Marshal(map[string]interface{}{

workspace-server/internal/handlers/mcp_test.go

@@ -26,6 +26,10 @@ func newMCPHandler(t *testing.T) (*MCPHandler, sqlmock.Sqlmock) {
 	t.Helper()
 	mock := setupTestDB(t)
 	h := NewMCPHandler(db.DB, newTestBroadcaster())
+	// Wire memv2 so toolCommitMemory takes the v2 path (where GLOBAL scope
+	// is blocked before any DB call), rather than the legacy shim path
+	// (which calls scopeToWritableNamespace before the scope check).
+	h.withMemoryV2APIs(nil, nil)
 	return h, mock
 }
 

workspace-server/internal/handlers/org.go

@@ -697,6 +697,31 @@ func (h *OrgHandler) Import(c *gin.Context) {
 			})
 			return
 		}
+
+		// Per-workspace RequiredEnv preflight: checks that every RequiredEnv
+		// declared at the workspace level is covered by either (a) a global
+		// secret key (already validated above) or (b) a key present in the
+		// workspace's on-disk .env files (org root .env + per-workspace
+		// <files_dir>/.env). If neither covers the key the workspace is
+		// imported NOT CONFIGURED, which silently breaks the workspace at
+		// start time — the container boots without the required credential
+		// and every LLM call 401s or fails silently.  Issue #232.
+		// orgBaseDir is empty when importing via body.Template (inline YAML);
+		// in that case we cannot check .env files, so we skip this check
+		// and fall back to the global-only gate above (which correctly
+		// rejects any strict requirement not covered by global_secrets).
+		if orgBaseDir != "" {
+			wsMissing := collectPerWorkspaceUnsatisfied(tmpl.Workspaces, orgBaseDir, configured)
+			if len(wsMissing) > 0 {
+				c.JSON(http.StatusPreconditionFailed, gin.H{
+					"error":            "missing per-workspace required environment variables",
+					"missing_workspace_env": wsMissing,
+					"template":         tmpl.Name,
+					"suggestion":       "add these keys to the workspace's .env file or set them as global secrets before importing",
+				})
+				return
+			}
+		}
 	}
 
 	results := []map[string]interface{}{}

workspace-server/internal/handlers/org_external.go

@@ -346,7 +346,7 @@ func (g *gitFetcher) Fetch(ctx context.Context, rootDir, host, repoPath, ref str
 	// MkdirTemp creates the dir; git clone refuses to clone into a
 	// non-empty dir. Remove + recreate empty.
 	os.RemoveAll(tmpDir)
-	cloneAndConfig := append(gitArgs("clone", "--quiet", "--depth=1", "-b", ref, cloneURL, tmpDir))
+	cloneAndConfig := gitArgs("clone", "--quiet", "--depth=1", "-b", ref, cloneURL, tmpDir)
 	cmd := exec.CommandContext(ctx, "git", cloneAndConfig...)
 	cmd.Env = append(os.Environ(), "GIT_TERMINAL_PROMPT=0")
 	if out, err := cmd.CombinedOutput(); err != nil {

workspace-server/internal/handlers/org_import.go

@@ -941,6 +941,65 @@ func flattenAndSortRequirements(by map[string]EnvRequirement) []EnvRequirement {
 // can investigate.
 const globalSecretsPreflightLimit = 10000
 
+// PerWorkspaceUnsatisfied describes one per-workspace RequiredEnv that is
+// not covered by either a global secret or a key present in the
+// corresponding .env file.
+type PerWorkspaceUnsatisfied struct {
+	Workspace   string         `json:"workspace"`
+	FilesDir    string         `json:"files_dir,omitempty"`
+	Unsatisfied EnvRequirement `json:"unsatisfied_env"`
+}
+
+// collectPerWorkspaceUnsatisfied recursively walks workspaces and returns
+// per-workspace RequiredEnv entries that are not covered by (a) a global
+// secret key or (b) a key present in the workspace's .env file(s) (org root
+// .env + per-workspace <files_dir>/.env). This complements
+// collectOrgEnv + loadConfiguredGlobalSecretKeys, which together only
+// validate global-level RequiredEnv against global_secrets. The .env
+// lookup mirrors the runtime resolution in createWorkspaceTree so that
+// the preflight result matches what the container actually receives at
+// start time.
+func collectPerWorkspaceUnsatisfied(workspaces []OrgWorkspace, orgBaseDir string, globalSecrets map[string]struct{}) []PerWorkspaceUnsatisfied {
+	var out []PerWorkspaceUnsatisfied
+	var walk func([]OrgWorkspace)
+	walk = func(wsList []OrgWorkspace) {
+		for _, ws := range wsList {
+			// Build the set of keys available to this workspace from .env.
+			// This is the same three-source stack that createWorkspaceTree
+			// injects into the container:
+			//   1. Org root .env (parseEnvFile, no filesDir)
+			//   2. Workspace <files_dir>/.env (if filesDir is set)
+			//   3. Persona bootstrap env (MOLECULE_PERSONA_ROOT/<filesDir>/env)
+			// Items 1+2 are on-disk and testable; item 3 is host-only and
+			// skipped here (persona env does NOT satisfy required_env —
+			// it carries identity tokens, not workspace LLM keys).
+			envFromFiles := loadWorkspaceEnv(orgBaseDir, ws.FilesDir)
+			// Convert map[string]string (from .env files) to map[string]struct{}
+			// to match IsSatisfied's signature.
+			envSet := make(map[string]struct{}, len(envFromFiles))
+			for k := range envFromFiles {
+				envSet[k] = struct{}{}
+			}
+			for _, req := range ws.RequiredEnv {
+				if req.IsSatisfied(globalSecrets) {
+					continue // covered by a global secret
+				}
+				if req.IsSatisfied(envSet) {
+					continue // covered by a per-workspace .env file
+				}
+				out = append(out, PerWorkspaceUnsatisfied{
+					Workspace:   ws.Name,
+					FilesDir:    ws.FilesDir,
+					Unsatisfied: req,
+				})
+			}
+			walk(ws.Children)
+		}
+	}
+	walk(workspaces)
+	return out
+}
+
 func loadConfiguredGlobalSecretKeys(ctx context.Context) (map[string]struct{}, error) {
 	rows, err := db.DB.QueryContext(ctx,
 		`SELECT key FROM global_secrets WHERE octet_length(encrypted_value) > 0 LIMIT $1`,

workspace-server/internal/handlers/org_workspace_required_env_test.go

@@ -0,0 +1,226 @@
+package handlers
+
+import (
+	"os"
+	"path/filepath"
+	"testing"
+)
+
+// TestCollectPerWorkspaceUnsatisfied_BothFiles covers the case where a key
+// is present in both the org root .env and the workspace-specific .env. Both
+// should satisfy the requirement (no entry in output).
+func TestCollectPerWorkspaceUnsatisfied_BothFiles(t *testing.T) {
+	tmp := t.TempDir()
+	writeEnvFile(t, tmp, ".env", "PER_WS_KEY=globalvalue")
+	writeEnvFile(t, tmp, "ws-a/.env", "PER_WS_KEY=wsvalue")
+
+	workspaces := []OrgWorkspace{
+		{Name: "ws-a", FilesDir: "ws-a", RequiredEnv: []EnvRequirement{{Name: "PER_WS_KEY"}}},
+	}
+
+	// Global secret covers it.
+	globals := map[string]struct{}{"PER_WS_KEY": {}}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 0 {
+		t.Errorf("PER_WS_KEY present in global + .env: should be satisfied, got %d missing", len(missing))
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_WorkspaceEnvOnly covers a key present
+// only in the workspace-specific .env file (not global). Should be satisfied.
+func TestCollectPerWorkspaceUnsatisfied_WorkspaceEnvOnly(t *testing.T) {
+	tmp := t.TempDir()
+	writeEnvFile(t, tmp, "dev-lead/.env", "WORKSPACE_KEY=val")
+
+	workspaces := []OrgWorkspace{
+		{Name: "Dev Lead", FilesDir: "dev-lead", RequiredEnv: []EnvRequirement{{Name: "WORKSPACE_KEY"}}},
+	}
+
+	globals := map[string]struct{}{} // nothing in global
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 0 {
+		t.Errorf("WORKSPACE_KEY in ws .env only: should be satisfied, got %d missing", len(missing))
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_OrgRootEnvOnly covers a key present
+// only in the org root .env file (not per-workspace). Should be satisfied.
+func TestCollectPerWorkspaceUnsatisfied_OrgRootEnvOnly(t *testing.T) {
+	tmp := t.TempDir()
+	writeEnvFile(t, tmp, ".env", "ORG_ROOT_KEY=val")
+
+	workspaces := []OrgWorkspace{
+		{Name: "ws-b", FilesDir: "ws-b", RequiredEnv: []EnvRequirement{{Name: "ORG_ROOT_KEY"}}},
+	}
+
+	globals := map[string]struct{}{}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 0 {
+		t.Errorf("ORG_ROOT_KEY in org root .env only: should be satisfied, got %d missing", len(missing))
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_GlobalCovers checks that a global
+// secret alone satisfies a per-workspace RequiredEnv even when the .env
+// files don't have the key.
+func TestCollectPerWorkspaceUnsatisfied_GlobalCovers(t *testing.T) {
+	tmp := t.TempDir()
+	// No .env files at all.
+
+	workspaces := []OrgWorkspace{
+		{Name: "ws-c", RequiredEnv: []EnvRequirement{{Name: "GLOBAL_COVERED"}}},
+	}
+
+	globals := map[string]struct{}{"GLOBAL_COVERED": {}}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 0 {
+		t.Errorf("GLOBAL_COVERED satisfied by global: should be satisfied, got %d missing", len(missing))
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_Missing covers the core bug: a
+// RequiredEnv declared at the workspace level where the key is absent from
+// both global_secrets and the .env file. The import MUST return 412.
+func TestCollectPerWorkspaceUnsatisfied_Missing(t *testing.T) {
+	tmp := t.TempDir()
+	// No .env files at all.
+
+	workspaces := []OrgWorkspace{
+		{Name: "Dev Lead", FilesDir: "dev-lead", RequiredEnv: []EnvRequirement{{Name: "MISSING_REQUIRED_KEY"}}},
+	}
+
+	globals := map[string]struct{}{} // no global secret
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 1 {
+		t.Fatalf("expected 1 missing entry, got %d", len(missing))
+	}
+	if missing[0].Workspace != "Dev Lead" {
+		t.Errorf("expected workspace 'Dev Lead', got %q", missing[0].Workspace)
+	}
+	if missing[0].Unsatisfied.Name != "MISSING_REQUIRED_KEY" {
+		t.Errorf("expected unsatisfied key 'MISSING_REQUIRED_KEY', got %q", missing[0].Unsatisfied.Name)
+	}
+	if missing[0].FilesDir != "dev-lead" {
+		t.Errorf("expected files_dir 'dev-lead', got %q", missing[0].FilesDir)
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_AnyOfGroup covers an any-of group where
+// none of the alternatives are present in global or .env. Should report
+// the group as unsatisfied.
+func TestCollectPerWorkspaceUnsatisfied_AnyOfGroup(t *testing.T) {
+	tmp := t.TempDir()
+
+	workspaces := []OrgWorkspace{
+		{
+			Name:     "Claude Bot",
+			FilesDir: "claude-bot",
+			RequiredEnv: []EnvRequirement{
+				{AnyOf: []string{"ANTHROPIC_API_KEY", "CLAUDE_CODE_OAUTH_TOKEN"}},
+			},
+		},
+	}
+
+	globals := map[string]struct{}{}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 1 {
+		t.Fatalf("expected 1 missing any-of entry, got %d", len(missing))
+	}
+	if missing[0].Workspace != "Claude Bot" {
+		t.Errorf("expected workspace 'Claude Bot', got %q", missing[0].Workspace)
+	}
+	if len(missing[0].Unsatisfied.AnyOf) != 2 {
+		t.Errorf("expected any-of group with 2 members, got %v", missing[0].Unsatisfied.AnyOf)
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_NestedChildren covers grandchildren
+// workspaces that also declare RequiredEnv. The recursive walk must visit
+// children and grandchildren.
+func TestCollectPerWorkspaceUnsatisfied_NestedChildren(t *testing.T) {
+	tmp := t.TempDir()
+
+	workspaces := []OrgWorkspace{
+		{
+			Name: "Root",
+			Children: []OrgWorkspace{
+				{
+					Name: "Child",
+					Children: []OrgWorkspace{
+						{Name: "Grandchild", FilesDir: "grandchild", RequiredEnv: []EnvRequirement{{Name: "DEEP_KEY"}}},
+					},
+				},
+			},
+		},
+	}
+
+	globals := map[string]struct{}{}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 1 {
+		t.Fatalf("expected 1 missing entry from grandchild, got %d", len(missing))
+	}
+	if missing[0].Workspace != "Grandchild" {
+		t.Errorf("expected 'Grandchild', got %q", missing[0].Workspace)
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_EmptyOrgBaseDir covers the case where
+// orgBaseDir is empty (inline template import). No .env files can be
+// checked, so missing keys cannot be attributed to .env absence. The
+// function should NOT crash and should only report entries satisfiable
+// by global (all missing since globals is empty).
+func TestCollectPerWorkspaceUnsatisfied_EmptyOrgBaseDir(t *testing.T) {
+	workspaces := []OrgWorkspace{
+		{Name: "ws-x", RequiredEnv: []EnvRequirement{{Name: "KEY_X"}}},
+	}
+
+	globals := map[string]struct{}{}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, "", globals)
+
+	// With no orgBaseDir and no global, KEY_X must be reported missing.
+	if len(missing) != 1 {
+		t.Errorf("expected 1 missing with empty orgBaseDir, got %d", len(missing))
+	}
+}
+
+// TestCollectPerWorkspaceUnsatisfied_MultipleWorkspaces reports only the
+// workspace whose RequiredEnv is unsatisfied, not the whole batch.
+func TestCollectPerWorkspaceUnsatisfied_MultipleWorkspaces(t *testing.T) {
+	tmp := t.TempDir()
+	writeEnvFile(t, tmp, "ws-ok/.env", "OK_KEY=val")
+
+	workspaces := []OrgWorkspace{
+		{Name: "ws-ok", FilesDir: "ws-ok", RequiredEnv: []EnvRequirement{{Name: "OK_KEY"}}},
+		{Name: "ws-missing", FilesDir: "ws-missing", RequiredEnv: []EnvRequirement{{Name: "BAD_KEY"}}},
+	}
+
+	globals := map[string]struct{}{}
+	missing := collectPerWorkspaceUnsatisfied(workspaces, tmp, globals)
+
+	if len(missing) != 1 {
+		t.Errorf("expected exactly 1 missing (BAD_KEY), got %d", len(missing))
+	}
+	if missing[0].Workspace != "ws-missing" {
+		t.Errorf("expected missing workspace 'ws-missing', got %q", missing[0].Workspace)
+	}
+}
+
+// writeEnvFile is a test helper that creates a .env file at the given path
+// with the given content.
+func writeEnvFile(t *testing.T, baseDir, relPath, content string) {
+	t.Helper()
+	fullPath := filepath.Join(baseDir, relPath)
+	if err := os.MkdirAll(filepath.Dir(fullPath), 0755); err != nil {
+		t.Fatalf("mkdirAll: %v", err)
+	}
+	if err := os.WriteFile(fullPath, []byte(content), 0644); err != nil {
+		t.Fatalf("writeFile %s: %v", fullPath, err)
+	}
+}

workspace-server/internal/pendinguploads/export_test.go

@@ -12,8 +12,8 @@ import (
 // time. The Go convention `export_test.go` keeps this seam OUT of the
 // production binary — files ending in _test.go are stripped at build
 // time, so this re-export only exists during `go test`.
-func StartSweeperWithIntervalForTest(ctx context.Context, storage Storage, ackRetention, interval time.Duration) {
-	startSweeperWithInterval(ctx, storage, ackRetention, interval, nil)
+func StartSweeperWithIntervalForTest(ctx context.Context, storage Storage, ackRetention, interval time.Duration, done chan struct{}) {
+	startSweeperWithInterval(ctx, storage, ackRetention, interval, done)
 }
 
 // StartSweeperForTest starts the sweeper and returns a done channel

workspace-server/internal/pendinguploads/sweeper_test.go

@@ -190,7 +190,14 @@ func TestStartSweeperWithInterval_TickerFiresAdditionalCycles(t *testing.T) {
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
-	done := pendinguploads.StartSweeperForTest(ctx, store, time.Hour)
+	// Use a short ticker interval (100ms) so the test runs fast without
+	// burning real wall-clock time. StartSweeperWithIntervalForTest is the
+	// test-friendly variant that accepts a caller-specified interval; the
+	// production SweepInterval of 5m is too coarse for a 2s deadline on
+	// a loaded CI runner (the ticker may not fire at all under CPU
+	// contention — the root cause of the pre-existing CI flake).
+	done := make(chan struct{})
+	go pendinguploads.StartSweeperWithIntervalForTest(ctx, store, time.Hour, 100*time.Millisecond, done)
 	// Immediate cycle + at least one tick-driven cycle.
 	store.waitForCycle(t, 2, 2*time.Second)