2026-06-13 22:52:34 +00:00
4 changed files with 114 additions and 25 deletions
@@ -70,8 +70,10 @@ FAIL=0
 WORK_DIR="$(mktemp -d)"
 WS_TARGET=""
 WS_SIBLING=""
+WS_PARENT=""
 WS_TARGET_TOK=""
 WS_SIBLING_TOK=""
+WS_PARENT_TOK=""
 MOCK_PID=""

 ADMIN_BEARER="${MOLECULE_ADMIN_TOKEN:-${ADMIN_TOKEN:-}}"
@@ -116,8 +118,9 @@ cleanup() {
    wait "$MOCK_PID" 2>/dev/null
  fi
  # Hard-purge any workspaces we created so repeat runs are deterministic.
-  for pair in "$WS_TARGET|$WS_TARGET_TOK|e2e-chan-target" \
-              "$WS_SIBLING|$WS_SIBLING_TOK|e2e-chan-sibling"; do
+  for pair in "$WS_TARGET|$WS_TARGET_TOK|e2e-chan-target-$$" \
+              "$WS_SIBLING|$WS_SIBLING_TOK|e2e-chan-sibling-$$" \
+              "$WS_PARENT|$WS_PARENT_TOK|e2e-chan-parent-$$"; do
    local wid tok name
    wid="${pair%%|*}"; pair="${pair#*|}"
    tok="${pair%%|*}"; name="${pair#*|}"
@@ -252,10 +255,18 @@ PY
 json_field() { python3 -c "import sys,json; print(json.load(sys.stdin).get('$1',''))"; }

 create_external_ws() {
-  local name="$1" resp wid
+  local name="$1" parent="${2:-}" resp wid parent_field=""
+  # core#2697: when no explicit parent is given, the server now defaults a new
+  # workspace's parent to the org's platform-agent root, or — absent one — the
+  # SOLE plain root. This test needs target + sibling to be genuine SIBLINGS
+  # (so purging the target must NOT cascade to the sibling), so callers pass an
+  # explicit shared parent. Without it the 2nd no-parent create would nest
+  # under the 1st (the sole root) and the purge-over-reach assertion would
+  # spuriously fail on the new default-parent behavior.
+  [ -n "$parent" ] && parent_field=",\"parent_id\":\"$parent\""
  resp=$(curl -s -X POST "$BASE/workspaces" "${ADMIN_AUTH[@]}" \
    -H "Content-Type: application/json" \
-    -d "{\"name\":\"$name\",\"runtime\":\"external\",\"external\":true,\"tier\":1}")
+    -d "{\"name\":\"$name\",\"runtime\":\"external\",\"external\":true,\"tier\":1$parent_field}")
  wid=$(printf '%s' "$resp" | json_field id)
  if [ -z "$wid" ]; then
    echo "FATAL: could not create workspace $name: $resp" >&2
@@ -280,9 +291,14 @@ fi
 start_mock

 # ── workspaces ──────────────────────────────────────────────────────────
-IFS=$'\t' read -r WS_TARGET WS_TARGET_TOK < <(create_external_ws "e2e-chan-target-$$")
-IFS=$'\t' read -r WS_SIBLING WS_SIBLING_TOK < <(create_external_ws "e2e-chan-sibling-$$")
-echo "target=$WS_TARGET sibling=$WS_SIBLING"
+# Create a common parent first, then nest target + sibling under it as genuine
+# siblings. This keeps the purge-over-reach invariant (purging target must not
+# touch sibling) independent of the core#2697 default-parent behavior, which
+# would otherwise nest the 2nd no-parent create under the 1st (the sole root).
+IFS=$'\t' read -r WS_PARENT WS_PARENT_TOK < <(create_external_ws "e2e-chan-parent-$$")
+IFS=$'\t' read -r WS_TARGET WS_TARGET_TOK < <(create_external_ws "e2e-chan-target-$$" "$WS_PARENT")
+IFS=$'\t' read -r WS_SIBLING WS_SIBLING_TOK < <(create_external_ws "e2e-chan-sibling-$$" "$WS_PARENT")
+echo "parent=$WS_PARENT target=$WS_TARGET sibling=$WS_SIBLING"

 WS_AUTH=("${ADMIN_AUTH[@]}")
 [ -n "$WS_TARGET_TOK" ] && WS_AUTH=(-H "Authorization: Bearer $WS_TARGET_TOK")
@@ -202,21 +202,50 @@ const conciergeDeclaredModel = "moonshot/kimi-k2.6"
 // sufficient and stable across restarts.
 var SelfHostedPlatformAgentID = uuid.NewSHA1(uuid.NameSpaceURL, []byte("molecule:self-hosted:platform-agent")).String()

-// platformRootWorkspaceID returns the org's single kind='platform' root
-// workspace id, or "" when there is none (pre-install / bootstrap) or more
-// than one (ambiguous — multi-org self-host DB; fail soft, change nothing).
-// Used by the workspace-create path to default parent_id (core#2609): a
-// workspace created without an explicit parent must land UNDER the org root,
-// not beside it — a parent_id-NULL orphan is outside the org subtree, so the
-// concierge cannot reach it over A2A ("cannot communicate per hierarchy
-// rules") and the canvas renders it depth-1 next to the root (#2601).
-func platformRootWorkspaceID(ctx context.Context) string {
-	rows, err := db.DB.QueryContext(ctx,
+// defaultCreateParentID resolves the parent a NEW workspace should nest under
+// when the caller didn't pass one. Order:
+//  1. The org's platform-agent root (kind='platform'), if exactly one — the
+//     intended home (core#2609).
+//  2. FALLBACK (core#2697): when the org has NO platform-agent (e.g. a tenant
+//     provisioned with only a plain root workspace — JRS had just its SEO Agent
+//     at parent_id NULL), nest under the SOLE non-removed root workspace if there
+//     is exactly one. Without this, new workspaces scatter at bare root as
+//     siblings of that root agent, and approval/discovery treat each NULL-parent
+//     row as its own org root, breaking hierarchy + delegation routing.
+//
+// The root fallback fires ONLY for the ZERO-platform case. When MULTIPLE
+// platform agents exist (ambiguous), we preserve the original fail-soft
+// behavior and return "" WITHOUT falling back to a root — picking a root there
+// would silently change the intended ambiguous-platform semantics (CR2 #2783).
+// Returns "" when: >1 platform; or 0 platform with 0/>1 roots — preserving
+// bootstrap/self-host multi-root behavior. The DURABLE fix is guaranteeing every
+// org has a platform-agent at provision; this is the safe runtime fallback.
+func defaultCreateParentID(ctx context.Context) string {
+	// Count platform-agent roots directly (LIMIT 2 distinguishes 0 / 1 / >1).
+	plats := queryUpToTwoIDs(ctx,
 		`SELECT id FROM workspaces WHERE COALESCE(kind, 'workspace') = 'platform' AND status != 'removed' LIMIT 2`)
+	if len(plats) == 1 {
+		return plats[0] // the platform-agent root (core#2609)
+	}
+	if len(plats) >= 2 {
+		return "" // ambiguous platform — fail soft, do NOT fall back to a root
+	}
+	// Exactly ZERO platform agents → nest under the sole plain root if unambiguous.
+	roots := queryUpToTwoIDs(ctx,
+		`SELECT id FROM workspaces WHERE parent_id IS NULL AND status != 'removed' LIMIT 2`)
+	if len(roots) == 1 {
+		return roots[0]
+	}
+	return ""
+}
+
+// queryUpToTwoIDs runs an id-selecting query (with its own LIMIT 2) and returns
+// up to two ids; on any error it returns an empty slice (fail-soft — defaulting
+// the parent is best-effort and must never fail the create).
+func queryUpToTwoIDs(ctx context.Context, query string) []string {
+	rows, err := db.DB.QueryContext(ctx, query)
 	if err != nil {
-		// Fail soft: defaulting the parent is best-effort; create must not
-		// fail because the root lookup hiccuped.
-		return ""
+		return nil
 	}
 	defer rows.Close()
 	ids := make([]string, 0, 2)
@@ -226,10 +255,7 @@ func platformRootWorkspaceID(ctx context.Context) string {
 			ids = append(ids, id)
 		}
 	}
-	if len(ids) == 1 {
-		return ids[0]
-	}
-	return ""
+	return ids
 }

 // defaultPlatformAgentName returns the display name for the org's platform
@@ -651,3 +651,45 @@ func TestConciergeSystemPrompt_IncludesAckFirstDirective(t *testing.T) {
 		t.Errorf("concierge prompt missing 'Report back clearly' section header (the ack-first directive should live in this section per core#2724):\n%s", prompt)
 	}
 }
+
+// TestDefaultCreateParentID covers core#2697: new workspaces nest under the
+// platform-agent root when one exists, else fall back to the SOLE plain root
+// workspace (the JRS case — a lone SEO Agent at parent_id NULL), else "".
+func TestDefaultCreateParentID(t *testing.T) {
+	platQ := `SELECT id FROM workspaces WHERE COALESCE\(kind, 'workspace'\) = 'platform'`
+	rootQ := `SELECT id FROM workspaces WHERE parent_id IS NULL`
+
+	t.Run("prefers the platform-agent root when exactly one exists", func(t *testing.T) {
+		mock := setupTestDB(t)
+		mock.ExpectQuery(platQ).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("plat-1"))
+		if got := defaultCreateParentID(context.Background()); got != "plat-1" {
+			t.Fatalf("want plat-1, got %q", got)
+		}
+	})
+
+	t.Run("falls back to the sole plain root when no platform-agent (JRS case)", func(t *testing.T) {
+		mock := setupTestDB(t)
+		mock.ExpectQuery(platQ).WillReturnRows(sqlmock.NewRows([]string{"id"})) // 0 platform
+		mock.ExpectQuery(rootQ).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("seo-agent"))
+		if got := defaultCreateParentID(context.Background()); got != "seo-agent" {
+			t.Fatalf("want seo-agent (fallback to sole root), got %q", got)
+		}
+	})
+
+	t.Run("returns empty when no platform and multiple roots (ambiguous)", func(t *testing.T) {
+		mock := setupTestDB(t)
+		mock.ExpectQuery(platQ).WillReturnRows(sqlmock.NewRows([]string{"id"}))
+		mock.ExpectQuery(rootQ).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("r1").AddRow("r2"))
+		if got := defaultCreateParentID(context.Background()); got != "" {
+			t.Fatalf("want empty (ambiguous multi-root), got %q", got)
+		}
+	})
+	t.Run("returns empty when MULTIPLE platform agents (ambiguous) — no root fallback (CR2 #2783)", func(t *testing.T) {
+		mock := setupTestDB(t)
+		// >1 platform → fail-soft empty; the root query must NOT run.
+		mock.ExpectQuery(platQ).WillReturnRows(sqlmock.NewRows([]string{"id"}).AddRow("p1").AddRow("p2"))
+		if got := defaultCreateParentID(context.Background()); got != "" {
+			t.Fatalf("want empty (ambiguous multi-platform must NOT fall back to a root), got %q", got)
+		}
+	})
+}
@@ -587,8 +587,13 @@ func (h *WorkspaceHandler) Create(c *gin.Context) {
 	// every delegation died with "cannot communicate per hierarchy rules".
 	// Best-effort: no platform root (or an ambiguous >1) leaves NULL intact,
 	// preserving bootstrap/self-host multi-root behavior.
+	//
+	// core#2697: defaultCreateParentID also falls back to the SOLE plain root
+	// workspace when the org has no platform-agent — so tenants provisioned
+	// without a concierge (e.g. JRS's lone SEO Agent) nest new workspaces under
+	// that root instead of scattering them as bare-root siblings.
 	if payload.ParentID == nil {
-		if rootID := platformRootWorkspaceID(ctx); rootID != "" {
+		if rootID := defaultCreateParentID(ctx); rootID != "" {
 			payload.ParentID = &rootID
 		}
 	}