Addressing PR feedback

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

.github/workflows/bashlib.sh

@@ -59,15 +59,6 @@ build-assets() {
   say "::endgroup::"
 }
 
-build-embedded-sdk() {
-  cd "$GITHUB_WORKSPACE/superset-embedded-sdk"
-
-  say "::group::Build embedded SDK bundle for E2E tests"
-  npm ci
-  npm run build
-  say "::endgroup::"
-}
-
 build-instrumented-assets() {
   cd "$GITHUB_WORKSPACE/superset-frontend"
 

.github/workflows/claude.yml

@@ -17,13 +17,12 @@ jobs:
     steps:
       - name: Check if user is allowed
         id: check
+        env:
+          COMMENTER: ${{ github.event.comment.user.login }}
         run: |
           # List of allowed users
           ALLOWED_USERS="mistercrunch,rusackas"
 
-          # Get the commenter's username
-          COMMENTER="${{ github.event.comment.user.login }}"
-
           echo "Checking permissions for user: $COMMENTER"
 
           # Check if user is in allowed list
@@ -45,9 +44,12 @@ jobs:
     steps:
       - name: Comment access denied
         uses: actions/github-script@3a2844b7e9c422d3c10d287c895573f7108da1b3 # v9.0.0
+        env:
+          COMMENTER_LOGIN: ${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}
         with:
           script: |
-            const message = `👋 Hi @${{ github.event.comment.user.login || github.event.review.user.login || github.event.issue.user.login }}!
+            const commenter = process.env.COMMENTER_LOGIN;
+            const message = `👋 Hi @${commenter}!
 
             Thanks for trying to use Claude Code, but currently only certain team members have access to this feature.
 

.github/workflows/codeql-analysis.yml

@@ -41,7 +41,7 @@ jobs:
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@v4
+        uses: github/codeql-action/init@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
         with:
           languages: ${{ matrix.language }}
           # If you wish to specify custom queries, you can do so here or in a config file.
@@ -53,6 +53,6 @@ jobs:
 
       - name: Perform CodeQL Analysis
         if: steps.check.outputs.python || steps.check.outputs.frontend
-        uses: github/codeql-action/analyze@v4
+        uses: github/codeql-action/analyze@68bde559dea0fdcac2102bfdf6230c5f70eb485e # v4
         with:
           category: "/language:${{matrix.language}}"

.github/workflows/superset-e2e.yml

@@ -169,7 +169,6 @@ jobs:
       PYTHONPATH: ${{ github.workspace }}
       REDIS_PORT: 16379
       GITHUB_TOKEN: ${{ github.token }}
-      SUPERSET_FEATURE_EMBEDDED_SUPERSET: "true"
     services:
       postgres:
         image: postgres:17-alpine
@@ -240,11 +239,6 @@ jobs:
         uses: ./.github/actions/cached-dependencies
         with:
           run: build-instrumented-assets
-      - name: Build embedded SDK
-        if: steps.check.outputs.python || steps.check.outputs.frontend
-        uses: ./.github/actions/cached-dependencies
-        with:
-          run: build-embedded-sdk
       - name: Install Playwright
         if: steps.check.outputs.python || steps.check.outputs.frontend
         uses: ./.github/actions/cached-dependencies

.github/workflows/superset-playwright.yml

@@ -43,7 +43,6 @@ jobs:
       PYTHONPATH: ${{ github.workspace }}
       REDIS_PORT: 16379
       GITHUB_TOKEN: ${{ github.token }}
-      SUPERSET_FEATURE_EMBEDDED_SUPERSET: "true"
     services:
       postgres:
         image: postgres:17-alpine
@@ -114,11 +113,6 @@ jobs:
         uses: ./.github/actions/cached-dependencies
         with:
           run: build-instrumented-assets
-      - name: Build embedded SDK
-        if: steps.check.outputs.python || steps.check.outputs.frontend
-        uses: ./.github/actions/cached-dependencies
-        with:
-          run: build-embedded-sdk
       - name: Install Playwright
         if: steps.check.outputs.python || steps.check.outputs.frontend
         uses: ./.github/actions/cached-dependencies

docker/pythonpath_dev/superset_config_docker_light.py

@@ -18,8 +18,6 @@
 # Configuration for docker-compose-light.yml - disables Redis and uses minimal services
 
 # Import all settings from the main config first
-import os
-
 from flask_caching.backends.filesystemcache import FileSystemCache
 
 from superset_config import *  # noqa: F403
@@ -38,31 +36,3 @@ THUMBNAIL_CACHE_CONFIG = CACHE_CONFIG
 
 # Disable Celery entirely for lightweight mode
 CELERY_CONFIG = None  # type: ignore[assignment,misc]
-
-# Honor SUPERSET_FEATURE_<NAME> env vars on top of any flags inherited from
-# superset_config. Lets local dev/e2e enable features (e.g. EMBEDDED_SUPERSET)
-# without editing shipped config files. Only the literal string "true"
-# (case-insensitive) is treated as enabled — "1"/"yes"/"on" are not, matching
-# the strict-string convention used elsewhere in Superset's env parsing.
-FEATURE_FLAGS = {
-    **FEATURE_FLAGS,  # noqa: F405
-    **{
-        name[len("SUPERSET_FEATURE_") :]: value.strip().lower() == "true"
-        for name, value in os.environ.items()
-        if name.startswith("SUPERSET_FEATURE_")
-    },
-}
-
-# Disable Talisman so /embedded/<uuid> doesn't return X-Frame-Options:SAMEORIGIN.
-# Without this, browsers refuse to render Superset inside an iframe from a
-# different origin (i.e. the embedded SDK use case). Production/CI configures
-# Talisman with explicit `frame-ancestors`; for the lightweight local stack we
-# just turn it off.
-TALISMAN_ENABLED = False
-
-# Guest tokens (used by the embedded SDK) inherit the "Public" role's perms.
-# Out of the box Public has zero perms, so embedded dashboards immediately fail
-# their first call (`/api/v1/me/roles/`) with 403. Mirror Public to Gamma —
-# the standard read-only viewer role — so the embedded flow can authenticate
-# and load dashboard data in local dev.
-PUBLIC_ROLE_LIKE = "Gamma"

superset-frontend/packages/superset-ui-core/src/components/Label/index.tsx

@@ -38,7 +38,7 @@ export function Label(props: LabelProps) {
   } = props;
 
   const baseColor = getColorVariants(theme, type);
-  const color = baseColor.active;
+  const color = baseColor.text;
   const borderColor = baseColor.border;
   const backgroundColor = baseColor.bg;
 

superset-frontend/playwright.config.ts

@@ -95,7 +95,6 @@ export default defineConfig({
       testIgnore: [
         '**/tests/auth/**/*.spec.ts',
         '**/tests/sqllab/**/*.spec.ts',
-        '**/tests/embedded/**/*.spec.ts',
         ...(process.env.INCLUDE_EXPERIMENTAL ? [] : ['**/experimental/**']),
       ],
       use: {
@@ -133,22 +132,6 @@ export default defineConfig({
         // No storageState = clean browser with no cached cookies
       },
     },
-    {
-      // Embedded dashboard tests - validates the full embedding flow:
-      // external app -> SDK -> iframe -> guest token -> dashboard render.
-      // Each spec file mutates per-dashboard embedding state (UUID,
-      // allowed_domains) on a single shared Superset, so files must not
-      // run in parallel even if more are added later.
-      name: 'chromium-embedded',
-      testMatch: '**/tests/embedded/**/*.spec.ts',
-      fullyParallel: false,
-      use: {
-        browserName: 'chromium',
-        testIdAttribute: 'data-test',
-        // Uses admin auth for API calls to configure embedding and get guest tokens
-        storageState: 'playwright/.auth/user.json',
-      },
-    },
   ],
 
   // Web server setup - disabled in CI (Flask started separately in workflow)

superset-frontend/playwright/embedded-app/index.html

@@ -1,96 +0,0 @@
-<!--
-Licensed to the Apache Software Foundation (ASF) under one
-or more contributor license agreements.  See the NOTICE file
-distributed with this work for additional information
-regarding copyright ownership.  The ASF licenses this file
-to you under the Apache License, Version 2.0 (the
-"License"); you may not use this file except in compliance
-with the License.  You may obtain a copy of the License at
-
-  http://www.apache.org/licenses/LICENSE-2.0
-
-Unless required by applicable law or agreed to in writing,
-software distributed under the License is distributed on an
-"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
-KIND, either express or implied.  See the License for the
-specific language governing permissions and limitations
-under the License.
--->
-<!DOCTYPE html>
-<html lang="en">
-<head>
-  <meta charset="UTF-8" />
-  <meta name="viewport" content="width=device-width, initial-scale=1.0" />
-  <title>Embedded Dashboard Test App</title>
-  <style>
-    html, body { margin: 0; padding: 0; height: 100%; }
-    #superset-container { width: 100%; height: 100vh; }
-    #superset-container iframe { width: 100%; height: 100%; border: none; }
-    #error { color: red; padding: 20px; display: none; }
-    #status { padding: 10px; font-family: monospace; font-size: 12px; }
-  </style>
-</head>
-<body>
-  <div id="status">Initializing embedded dashboard...</div>
-  <div id="error"></div>
-  <div id="superset-container" data-test="embedded-container"></div>
-
-  <script src="/sdk/index.js"></script>
-  <script>
-    (async function () {
-      const params = new URLSearchParams(window.location.search);
-      const uuid = params.get('uuid');
-      const supersetDomain = params.get('supersetDomain');
-
-      if (!uuid || !supersetDomain) {
-        document.getElementById('error').style.display = 'block';
-        document.getElementById('error').textContent =
-          'Missing required query params: uuid, supersetDomain';
-        return;
-      }
-
-      const statusEl = document.getElementById('status');
-
-      // fetchGuestToken is injected by Playwright via page.exposeFunction()
-      // Falls back to window.__guestToken for simple/static token injection
-      async function fetchGuestToken() {
-        if (typeof window.__fetchGuestToken === 'function') {
-          statusEl.textContent = 'Fetching guest token...';
-          const token = await window.__fetchGuestToken();
-          statusEl.textContent = 'Guest token received, loading dashboard...';
-          return token;
-        }
-        if (window.__guestToken) {
-          return window.__guestToken;
-        }
-        throw new Error('No guest token source available');
-      }
-
-      try {
-        // Parse optional UI config from query params
-        const uiConfig = {};
-        if (params.get('hideTitle') === 'true') uiConfig.hideTitle = true;
-        if (params.get('hideTab') === 'true') uiConfig.hideTab = true;
-        if (params.get('hideChartControls') === 'true') uiConfig.hideChartControls = true;
-
-        const dashboard = await supersetEmbeddedSdk.embedDashboard({
-          id: uuid,
-          supersetDomain: supersetDomain,
-          mountPoint: document.getElementById('superset-container'),
-          fetchGuestToken: fetchGuestToken,
-          dashboardUiConfig: Object.keys(uiConfig).length > 0 ? uiConfig : undefined,
-          debug: params.get('debug') === 'true',
-        });
-
-        statusEl.textContent = 'Dashboard embedded successfully';
-        // Expose dashboard API on window for Playwright assertions
-        window.__embeddedDashboard = dashboard;
-      } catch (err) {
-        document.getElementById('error').style.display = 'block';
-        document.getElementById('error').textContent = 'Embed failed: ' + err.message;
-        statusEl.textContent = 'Error';
-      }
-    })();
-  </script>
-</body>
-</html>

superset-frontend/playwright/helpers/api/dashboard.ts

@@ -132,14 +132,26 @@ export interface DashboardResult {
   published?: boolean;
 }
 
-async function getDashboardByFilter(
+/**
+ * Get a dashboard by its title
+ * @param page - Playwright page instance (provides authentication context)
+ * @param title - The dashboard_title to search for
+ * @returns Dashboard object if found, null if not found
+ */
+export async function getDashboardByName(
   page: Page,
-  col: 'dashboard_title' | 'slug',
-  value: string,
+  title: string,
 ): Promise<DashboardResult | null> {
-  const queryParam = rison.encode({
-    filters: [{ col, opr: 'eq', value }],
-  });
+  const filter = {
+    filters: [
+      {
+        col: 'dashboard_title',
+        opr: 'eq',
+        value: title,
+      },
+    ],
+  };
+  const queryParam = rison.encode(filter);
   const response = await apiGet(
     page,
     `${ENDPOINTS.DASHBOARD}?q=${queryParam}`,
@@ -157,29 +169,3 @@ async function getDashboardByFilter(
 
   return null;
 }
-
-/**
- * Get a dashboard by its title
- * @param page - Playwright page instance (provides authentication context)
- * @param title - The dashboard_title to search for
- * @returns Dashboard object if found, null if not found
- */
-export async function getDashboardByName(
-  page: Page,
-  title: string,
-): Promise<DashboardResult | null> {
-  return getDashboardByFilter(page, 'dashboard_title', title);
-}
-
-/**
- * Get a dashboard by its slug
- * @param page - Playwright page instance (provides authentication context)
- * @param slug - The slug to search for
- * @returns Dashboard object if found, null if not found
- */
-export async function getDashboardBySlug(
-  page: Page,
-  slug: string,
-): Promise<DashboardResult | null> {
-  return getDashboardByFilter(page, 'slug', slug);
-}

superset-frontend/playwright/helpers/api/embedded.ts

@@ -1,136 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { Page } from '@playwright/test';
-import { apiPost, apiPut, getCsrfToken } from './requests';
-import { ENDPOINTS as DASHBOARD_ENDPOINTS } from './dashboard';
-
-export const ENDPOINTS = {
-  SECURITY_LOGIN: 'api/v1/security/login',
-  GUEST_TOKEN: 'api/v1/security/guest_token/',
-} as const;
-
-export interface EmbeddedConfig {
-  uuid: string;
-  allowed_domains: string[];
-  dashboard_id: number;
-}
-
-/**
- * Enable embedding on a dashboard and return the embedded UUID.
- * Uses PUT (upsert) to preserve UUID across repeated calls.
- * @param page - Playwright page instance (provides authentication context)
- * @param dashboardIdOrSlug - Numeric dashboard id or slug
- * @param allowedDomains - Domains allowed to embed; empty array allows all
- * @returns Embedded config with UUID, allowed_domains, and dashboard_id
- */
-export async function apiEnableEmbedding(
-  page: Page,
-  dashboardIdOrSlug: number | string,
-  allowedDomains: string[] = [],
-): Promise<EmbeddedConfig> {
-  const response = await apiPut(
-    page,
-    `${DASHBOARD_ENDPOINTS.DASHBOARD}${dashboardIdOrSlug}/embedded`,
-    { allowed_domains: allowedDomains },
-  );
-  const body = await response.json();
-  return body.result as EmbeddedConfig;
-}
-
-/**
- * Login as admin and return the JWT access token used by the guest_token
- * endpoint. Cache the result at suite level (`beforeAll`) and pass it into
- * `getGuestToken` to avoid a fresh login on every test.
- *
- * Defaults match `playwright/global-setup.ts` so credentials come from one
- * source (env vars). Overrides via `options` win.
- */
-export async function getAccessToken(
-  page: Page,
-  options?: { username?: string; password?: string },
-): Promise<string> {
-  const username =
-    options?.username ?? process.env.PLAYWRIGHT_ADMIN_USERNAME ?? 'admin';
-  const password =
-    options?.password ?? process.env.PLAYWRIGHT_ADMIN_PASSWORD ?? 'general';
-  const loginResponse = await apiPost(
-    page,
-    ENDPOINTS.SECURITY_LOGIN,
-    {
-      username,
-      password,
-      provider: 'db',
-      refresh: true,
-    },
-    { allowMissingCsrf: true },
-  );
-  const loginBody = await loginResponse.json();
-  return loginBody.access_token;
-}
-
-/**
- * Get a guest token for an embedded dashboard.
- * If `accessToken` is provided, the login round-trip is skipped — preferred
- * for tests that fetch many tokens from a single suite.
- * @returns Signed guest token string
- */
-export async function getGuestToken(
-  page: Page,
-  dashboardId: number | string,
-  options?: {
-    accessToken?: string;
-    username?: string;
-    password?: string;
-    rls?: Array<{ dataset: number; clause: string }>;
-  },
-): Promise<string> {
-  const accessToken =
-    options?.accessToken ??
-    (await getAccessToken(page, {
-      username: options?.username,
-      password: options?.password,
-    }));
-  const rls = options?.rls ?? [];
-
-  // The guest_token endpoint authenticates via JWT Bearer, but if the
-  // request also carries a session cookie (which page.request inherits from
-  // storageState), Flask-WTF still requires a matching X-CSRFToken. Send it
-  // unconditionally so this works whether the caller is authenticated via
-  // session, JWT, or both.
-  const { token: csrfToken } = await getCsrfToken(page);
-  const guestResponse = await page.request.post(ENDPOINTS.GUEST_TOKEN, {
-    data: {
-      user: {
-        username: 'embedded_test_user',
-        first_name: 'Embedded',
-        last_name: 'TestUser',
-      },
-      resources: [{ type: 'dashboard', id: String(dashboardId) }],
-      rls,
-    },
-    headers: {
-      'Content-Type': 'application/json',
-      Authorization: `Bearer ${accessToken}`,
-      ...(csrfToken ? { 'X-CSRFToken': csrfToken } : {}),
-    },
-  });
-  const guestBody = await guestResponse.json();
-  return guestBody.token;
-}

superset-frontend/playwright/helpers/api/requests.ts

@@ -39,7 +39,7 @@ function getBaseUrl(): string {
   return url.endsWith('/') ? url : `${url}/`;
 }
 
-export interface CsrfResult {
+interface CsrfResult {
   token: string;
   error?: string;
 }
@@ -49,7 +49,7 @@ export interface CsrfResult {
  * Superset provides a CSRF token via api/v1/security/csrf_token/
  * The session cookie is automatically included by page.request
  */
-export async function getCsrfToken(page: Page): Promise<CsrfResult> {
+async function getCsrfToken(page: Page): Promise<CsrfResult> {
   try {
     const response = await page.request.get('api/v1/security/csrf_token/', {
       failOnStatusCode: false,

superset-frontend/playwright/pages/EmbeddedPage.ts

@@ -1,172 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { Page, FrameLocator, Locator, expect } from '@playwright/test';
-import { EMBEDDED } from '../utils/constants';
-
-/**
- * Page object for the embedded dashboard test app.
- *
- * The test app runs on a separate origin (its origin is assigned per-suite
- * via an OS-allocated port) and uses the @superset-ui/embedded-sdk to render
- * a Superset dashboard in an iframe. Playwright's page.exposeFunction()
- * bridges the guest token from Node.js into the browser page.
- */
-export class EmbeddedPage {
-  private readonly page: Page;
-
-  private static readonly SELECTORS = {
-    CONTAINER: '[data-test="embedded-container"]',
-    IFRAME: 'iframe[title="Embedded Dashboard"]',
-    STATUS: '#status',
-    ERROR: '#error',
-  } as const;
-
-  constructor(page: Page) {
-    this.page = page;
-  }
-
-  /**
-   * Set up the guest token bridge before navigating.
-   * Must be called BEFORE goto() since embedDashboard() calls fetchGuestToken
-   * immediately on page load.
-   */
-  async exposeTokenFetcher(tokenFn: () => Promise<string>): Promise<void> {
-    await this.page.exposeFunction('__fetchGuestToken', tokenFn);
-  }
-
-  /**
-   * Navigate to the embedded test app with the given parameters.
-   * `appUrl` is the origin of the static test app (assigned dynamically by
-   * the spec's beforeAll fixture so workers don't collide on a fixed port).
-   */
-  async goto(params: {
-    appUrl: string;
-    uuid: string;
-    supersetDomain: string;
-    hideTitle?: boolean;
-    hideTab?: boolean;
-    hideChartControls?: boolean;
-    debug?: boolean;
-  }): Promise<void> {
-    const searchParams = new URLSearchParams({
-      uuid: params.uuid,
-      supersetDomain: params.supersetDomain,
-    });
-    if (params.hideTitle) searchParams.set('hideTitle', 'true');
-    if (params.hideTab) searchParams.set('hideTab', 'true');
-    if (params.hideChartControls) searchParams.set('hideChartControls', 'true');
-    if (params.debug) searchParams.set('debug', 'true');
-
-    await this.page.goto(`${params.appUrl}/?${searchParams.toString()}`);
-  }
-
-  /**
-   * FrameLocator for the embedded dashboard iframe.
-   */
-  get iframe(): FrameLocator {
-    return this.page.frameLocator(EmbeddedPage.SELECTORS.IFRAME);
-  }
-
-  /**
-   * Wait for the iframe to appear in the DOM AND have its src set.
-   * The SDK appends the iframe element before assigning src, so a bare
-   * `state: 'attached'` wait races the src read.
-   */
-  async waitForIframe(options?: { timeout?: number }): Promise<void> {
-    const locator = this.page.locator(EmbeddedPage.SELECTORS.IFRAME);
-    await locator.waitFor({
-      state: 'attached',
-      timeout: options?.timeout ?? EMBEDDED.IFRAME_LOAD,
-    });
-    await expect(locator).toHaveAttribute('src', /.+/, {
-      timeout: options?.timeout ?? EMBEDDED.IFRAME_LOAD,
-    });
-  }
-
-  /**
-   * Wait for dashboard content to render inside the iframe.
-   * Looks for the grid-container which indicates charts are loading/loaded.
-   */
-  async waitForDashboardContent(options?: { timeout?: number }): Promise<void> {
-    const frame = this.iframe;
-    await frame
-      .locator('.grid-container, [data-test="grid-container"]')
-      .first()
-      .waitFor({
-        state: 'visible',
-        timeout: options?.timeout ?? EMBEDDED.DASHBOARD_RENDER,
-      });
-  }
-
-  /**
-   * Matches a chart cell that has finished loading: it contains a real viz
-   * element (svg, canvas, table) AND no longer hosts the `Loading` spinner
-   * (`data-test="loading-indicator"`). Excluding the spinner matters —
-   * the spinner itself renders an SVG, so a `:has(svg)`-only check can match
-   * a still-loading chart for the wrong reason.
-   */
-  static readonly RENDERED_CHART_SELECTOR =
-    '[data-test="chart-container"]:has(svg, canvas, table):not(:has([data-test="loading-indicator"]))';
-
-  /**
-   * Wait for at least one chart to finish rendering — viz drawn AND no
-   * loading spinner in that cell.
-   */
-  async waitForChartRendered(options?: { timeout?: number }): Promise<void> {
-    await this.iframe
-      .locator(EmbeddedPage.RENDERED_CHART_SELECTOR)
-      .first()
-      .waitFor({
-        state: 'visible',
-        timeout: options?.timeout ?? EMBEDDED.CHART_RENDER,
-      });
-  }
-
-  /**
-   * Locator for the dashboard title input inside the iframe.
-   * Returned as a `Locator` so callers can use `expect(...).toBeVisible()` /
-   * `.toBeHidden()` with auto-retry instead of one-shot `.isVisible()`.
-   */
-  get titleLocator(): Locator {
-    return this.iframe.locator(
-      '[data-test="dashboard-header-container"] [data-test="editable-title-input"]',
-    );
-  }
-
-  /**
-   * Get the status text from the test app.
-   */
-  async getStatus(): Promise<string> {
-    return (
-      (await this.page.locator(EmbeddedPage.SELECTORS.STATUS).textContent()) ??
-      ''
-    );
-  }
-
-  /**
-   * Get the error text, if any.
-   */
-  async getError(): Promise<string> {
-    const errorEl = this.page.locator(EmbeddedPage.SELECTORS.ERROR);
-    const display = await errorEl.evaluate(el => getComputedStyle(el).display);
-    if (display === 'none') return '';
-    return (await errorEl.textContent()) ?? '';
-  }
-}

superset-frontend/playwright/tests/embedded/embedded-dashboard.spec.ts

@@ -1,356 +0,0 @@
-/**
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *   http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing,
- * software distributed under the License is distributed on an
- * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
- * KIND, either express or implied.  See the License for the
- * specific language governing permissions and limitations
- * under the License.
- */
-
-import { test, expect, Browser, BrowserContext, Page } from '@playwright/test';
-import { createServer, IncomingMessage, ServerResponse, Server } from 'http';
-import { AddressInfo, Socket } from 'net';
-import { readFileSync, existsSync } from 'fs';
-import { join } from 'path';
-import {
-  apiEnableEmbedding,
-  getAccessToken,
-  getGuestToken,
-} from '../../helpers/api/embedded';
-import { getDashboardBySlug } from '../../helpers/api/dashboard';
-import { EmbeddedPage } from '../../pages/EmbeddedPage';
-
-/**
- * Superset domain (Flask server) — set by CI or defaults to local dev
- */
-const SUPERSET_DOMAIN = (() => {
-  const url = process.env.PLAYWRIGHT_BASE_URL || 'http://localhost:8088';
-  return url.replace(/\/+$/, '');
-})();
-
-const SUPERSET_BASE_URL = SUPERSET_DOMAIN.endsWith('/')
-  ? SUPERSET_DOMAIN
-  : `${SUPERSET_DOMAIN}/`;
-
-/**
- * Path to the SDK bundle built from superset-embedded-sdk/
- */
-const SDK_BUNDLE_PATH = join(
-  __dirname,
-  '../../../../superset-embedded-sdk/bundle/index.js',
-);
-
-/**
- * Path to the embedded test app static files
- */
-const EMBED_APP_DIR = join(__dirname, '../../embedded-app');
-
-/**
- * Create a minimal static file server for the embedded test app.
- * Serves only a fixed allowlist of routes — the test app references just
- * its index.html and the SDK bundle, so anything else is 404.
- */
-const INDEX_HTML_PATH = join(EMBED_APP_DIR, 'index.html');
-
-interface EmbedAppServer {
-  server: Server;
-  url: string;
-  close: () => Promise<void>;
-}
-
-/**
- * Start the static test app on an OS-assigned ephemeral port. Tracks open
- * sockets so close() doesn't hang on iframe keep-alive connections, and so
- * different workers/retries never collide on a fixed port.
- */
-async function startEmbedAppServer(): Promise<EmbedAppServer> {
-  const sockets = new Set<Socket>();
-  const server = createServer((req: IncomingMessage, res: ServerResponse) => {
-    const urlPath = req.url?.split('?')[0] || '/';
-
-    if (urlPath === '/sdk/index.js') {
-      if (!existsSync(SDK_BUNDLE_PATH)) {
-        res.writeHead(404);
-        res.end(
-          'SDK bundle not found. Run: cd superset-embedded-sdk && npm ci && npm run build',
-        );
-        return;
-      }
-      res.writeHead(200, { 'Content-Type': 'text/javascript' });
-      res.end(readFileSync(SDK_BUNDLE_PATH));
-      return;
-    }
-
-    if (urlPath === '/' || urlPath === '/index.html') {
-      res.writeHead(200, { 'Content-Type': 'text/html' });
-      res.end(readFileSync(INDEX_HTML_PATH));
-      return;
-    }
-
-    res.writeHead(404);
-    res.end('Not found');
-  });
-
-  server.on('connection', socket => {
-    sockets.add(socket);
-    socket.once('close', () => sockets.delete(socket));
-  });
-
-  await new Promise<void>((resolve, reject) => {
-    server.once('error', reject);
-    server.listen(0, '127.0.0.1', () => {
-      server.removeListener('error', reject);
-      resolve();
-    });
-  });
-
-  const address = server.address() as AddressInfo;
-  const url = `http://127.0.0.1:${address.port}`;
-
-  return {
-    server,
-    url,
-    close: () =>
-      new Promise<void>(resolve => {
-        for (const socket of sockets) socket.destroy();
-        sockets.clear();
-        server.close(() => resolve());
-      }),
-  };
-}
-
-/**
- * Create a browser context authenticated as admin for API-only work
- * (enabling embedding, restoring config). Caller is responsible for closing.
- */
-function createAdminContext(browser: Browser): Promise<BrowserContext> {
-  return browser.newContext({
-    storageState: 'playwright/.auth/user.json',
-    baseURL: SUPERSET_BASE_URL,
-  });
-}
-
-// ─── Test Suite ────────────────────────────────────────────────────────────
-
-// Describe wrapper is needed for shared server state and serial execution:
-// all tests share a static file server and must not run in parallel.
-test.describe('Embedded Dashboard E2E', () => {
-  test.describe.configure({ mode: 'serial' });
-
-  // The full embedded chain (login → guest token → iframe → dashboard render
-  // → chart render) routinely exceeds the 30s default on cold CI starts.
-  test.setTimeout(60000);
-
-  let appServer: EmbedAppServer;
-  let accessToken: string;
-  let embedUuid: string;
-  let dashboardId: number;
-
-  /**
-   * Set up a page to render the default embedded dashboard.
-   * Tests that need a different UUID or UI config should not use this helper.
-   */
-  async function setupEmbeddedPage(page: Page): Promise<EmbeddedPage> {
-    const embeddedPage = new EmbeddedPage(page);
-    await embeddedPage.exposeTokenFetcher(async () =>
-      getGuestToken(page, dashboardId, { accessToken }),
-    );
-    await embeddedPage.goto({
-      appUrl: appServer.url,
-      uuid: embedUuid,
-      supersetDomain: SUPERSET_DOMAIN,
-    });
-    await embeddedPage.waitForIframe();
-    await embeddedPage.waitForDashboardContent();
-    return embeddedPage;
-  }
-
-  test.beforeAll(async ({ browser }) => {
-    // Skip all tests if the SDK bundle hasn't been built
-    test.skip(
-      !existsSync(SDK_BUNDLE_PATH),
-      'Embedded SDK bundle not found. Build it with: cd superset-embedded-sdk && npm ci && npm run build',
-    );
-
-    appServer = await startEmbedAppServer();
-
-    // Use a fresh context with auth to set up test data via API
-    const context = await createAdminContext(browser);
-    const setupPage = await context.newPage();
-
-    try {
-      const dashboard = await getDashboardBySlug(setupPage, 'world_health');
-      if (!dashboard) {
-        throw new Error(
-          'Dashboard "world_health" not found. Ensure load_examples ran in CI setup.',
-        );
-      }
-      dashboardId = dashboard.id;
-
-      // Enable embedding on the dashboard (empty allowed_domains = allow all)
-      const embedded = await apiEnableEmbedding(setupPage, dashboardId);
-      embedUuid = embedded.uuid;
-
-      // Cache the JWT access token so tests don't re-login per guest token.
-      accessToken = await getAccessToken(setupPage);
-    } finally {
-      await context.close();
-    }
-  });
-
-  test.afterAll(async ({ browser }) => {
-    // Defensive restore in case the allowed_domains test failed mid-flight.
-    if (dashboardId !== undefined) {
-      const context = await createAdminContext(browser);
-      try {
-        const setupPage = await context.newPage();
-        await apiEnableEmbedding(setupPage, dashboardId, []);
-      } catch {
-        // Best-effort cleanup — never fail teardown.
-      } finally {
-        await context.close();
-      }
-    }
-
-    if (appServer) await appServer.close();
-  });
-
-  test('dashboard renders in embedded iframe', async ({ page }) => {
-    const embeddedPage = await setupEmbeddedPage(page);
-
-    // Verify the iframe src points to Superset's /embedded/ endpoint
-    await expect(
-      page.locator('iframe[title="Embedded Dashboard"]'),
-    ).toHaveAttribute('src', new RegExp(`/embedded/${embedUuid}`));
-
-    // Verify no errors in the test app
-    expect(await embeddedPage.getError()).toBe('');
-
-    // Baseline: title should be visible when hideTitle is not set. This
-    // doubles as a positive existence check the `hideTitle` test relies on
-    // for distinguishing "title was hidden" from "selector is wrong".
-    await expect(embeddedPage.titleLocator).toBeVisible();
-
-    // Prove the dashboard actually renders, not just the chrome.
-    await embeddedPage.waitForChartRendered();
-  });
-
-  test('UI config hideTitle hides dashboard title', async ({ page }) => {
-    const embeddedPage = new EmbeddedPage(page);
-    await embeddedPage.exposeTokenFetcher(async () =>
-      getGuestToken(page, dashboardId, { accessToken }),
-    );
-    await embeddedPage.goto({
-      appUrl: appServer.url,
-      uuid: embedUuid,
-      supersetDomain: SUPERSET_DOMAIN,
-      hideTitle: true,
-    });
-    await embeddedPage.waitForIframe();
-    await embeddedPage.waitForDashboardContent();
-
-    // The iframe URL should include uiConfig parameter
-    await expect(
-      page.locator('iframe[title="Embedded Dashboard"]'),
-    ).toHaveAttribute('src', /uiConfig=/);
-
-    // hideTitle removes the header from the DOM (rather than CSS-hiding it),
-    // so toBeHidden + toHaveCount(0) together assert: not visible AND
-    // confirmed-removed (so the test can't pass for the wrong reason if the
-    // selector ever drifts — the baseline test asserts the selector matches
-    // when hideTitle is off).
-    await expect(embeddedPage.titleLocator).toBeHidden();
-    await expect(embeddedPage.titleLocator).toHaveCount(0);
-  });
-
-  test('charts render inside embedded iframe', async ({ page }) => {
-    const embeddedPage = await setupEmbeddedPage(page);
-
-    await embeddedPage.waitForChartRendered();
-    const renderedCharts = embeddedPage.iframe.locator(
-      EmbeddedPage.RENDERED_CHART_SELECTOR,
-    );
-    expect(await renderedCharts.count()).toBeGreaterThan(0);
-  });
-
-  test('allowed_domains blocks unauthorized referrer', async ({
-    page,
-    browser,
-  }) => {
-    const context = await createAdminContext(browser);
-    const setupPage = await context.newPage();
-
-    try {
-      // Restrict to a domain that is NOT the test app's origin
-      const restrictedEmbed = await apiEnableEmbedding(setupPage, dashboardId, [
-        'https://allowed.example.com',
-      ]);
-
-      const embeddedPage = new EmbeddedPage(page);
-      await embeddedPage.exposeTokenFetcher(async () =>
-        getGuestToken(page, dashboardId, { accessToken }),
-      );
-
-      // The deterministic signal that the referrer check fired is the HTTP
-      // status of the /embedded/<uuid> response — assert that directly rather
-      // than racing against cross-origin iframe rendering.
-      const embeddedResponsePromise = page.waitForResponse(
-        resp =>
-          resp.url().includes(`/embedded/${restrictedEmbed.uuid}`) &&
-          resp.request().resourceType() === 'document',
-      );
-
-      await embeddedPage.goto({
-        appUrl: appServer.url,
-        uuid: restrictedEmbed.uuid,
-        supersetDomain: SUPERSET_DOMAIN,
-      });
-
-      const response = await embeddedResponsePromise;
-      expect(response.status()).toBe(403);
-    } finally {
-      // Restore the open embedding config for other tests in this file.
-      await apiEnableEmbedding(setupPage, dashboardId, []);
-      await context.close();
-    }
-  });
-
-  test('guest token enables dashboard data access', async ({ page }) => {
-    const embeddedPage = new EmbeddedPage(page);
-
-    let tokenCallCount = 0;
-    await embeddedPage.exposeTokenFetcher(async () => {
-      tokenCallCount += 1;
-      return getGuestToken(page, dashboardId, { accessToken });
-    });
-
-    await embeddedPage.goto({
-      appUrl: appServer.url,
-      uuid: embedUuid,
-      supersetDomain: SUPERSET_DOMAIN,
-    });
-    await embeddedPage.waitForIframe();
-    await embeddedPage.waitForDashboardContent();
-    await embeddedPage.waitForChartRendered();
-
-    // The SDK fetches the token exactly once per embed (caching is the
-    // SDK's responsibility, not ours) — assert the stronger invariant.
-    expect(tokenCallCount).toBe(1);
-
-    // Confirm at least one chart actually rendered with data, not just its shell
-    const renderedCharts = embeddedPage.iframe.locator(
-      EmbeddedPage.RENDERED_CHART_SELECTOR,
-    );
-    expect(await renderedCharts.count()).toBeGreaterThan(0);
-  });
-});

superset-frontend/playwright/utils/constants.ts

@@ -75,16 +75,3 @@ export const TIMEOUT = {
    */
   SLOW_TEST: 60000, // 60s for tests that chain multiple slow operations
 } as const;
-
-/**
- * Embedded dashboard test app configuration.
- * The test app is served by a Node.js http server started in the test fixture.
- */
-export const EMBEDDED = {
-  /** Timeout for iframe to appear in the DOM */
-  IFRAME_LOAD: 15000, // 15s
-  /** Timeout for dashboard content to render inside the iframe */
-  DASHBOARD_RENDER: 30000, // 30s
-  /** Timeout for individual chart cells to finish rendering */
-  CHART_RENDER: 30000, // 30s
-} as const;

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/htmlTextFilterValueGetter.test.ts

@@ -0,0 +1,83 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
+import htmlTextFilterValueGetter, {
+  htmlTextComparator,
+} from './htmlTextFilterValueGetter';
+
+const makeParams = (value: unknown): ValueGetterParams =>
+  ({
+    data: { foo: value },
+    colDef: { field: 'foo' },
+  }) as unknown as ValueGetterParams;
+
+test('htmlTextFilterValueGetter extracts visible text from HTML anchor', () => {
+  expect(
+    htmlTextFilterValueGetter(
+      makeParams(
+        '<a href="https://jira.example.com/123/S18_3232">S18_3232</a>',
+      ),
+    ),
+  ).toBe('S18_3232');
+});
+
+test('htmlTextFilterValueGetter strips nested HTML markup', () => {
+  expect(
+    htmlTextFilterValueGetter(
+      makeParams('<div><strong>Hello</strong> <em>World</em></div>'),
+    ),
+  ).toBe('Hello World');
+});
+
+test('htmlTextFilterValueGetter passes plain strings through', () => {
+  expect(htmlTextFilterValueGetter(makeParams('plain value'))).toBe(
+    'plain value',
+  );
+});
+
+test('htmlTextFilterValueGetter passes non-string values through', () => {
+  expect(htmlTextFilterValueGetter(makeParams(42))).toBe(42);
+  expect(htmlTextFilterValueGetter(makeParams(null))).toBeNull();
+  expect(htmlTextFilterValueGetter(makeParams(undefined))).toBeUndefined();
+});
+
+test('htmlTextComparator orders by visible text, not raw HTML', () => {
+  // URL prefixes (zzz vs bbb) would flip the order under raw-HTML sort,
+  // but the visible labels (S700_4002 vs S72_3212) sort the other way.
+  const left = '<a href="https://jira.example.com/zzz/S700_4002">S700_4002</a>';
+  const right = '<a href="https://jira.example.com/bbb/S72_3212">S72_3212</a>';
+  expect(htmlTextComparator(left, right)).toBeLessThan(0);
+});
+
+test('htmlTextComparator handles nulls and numbers', () => {
+  expect(htmlTextComparator(null, null)).toBe(0);
+  expect(htmlTextComparator(null, 'x')).toBeLessThan(0);
+  expect(htmlTextComparator('x', null)).toBeGreaterThan(0);
+  expect(htmlTextComparator(1, 2)).toBeLessThan(0);
+  expect(htmlTextComparator(2, 1)).toBeGreaterThan(0);
+});
+
+test('htmlTextComparator preserves default codepoint ordering for plain strings', () => {
+  // AG Grid's default string comparator orders by codepoint, so 'Z' (90)
+  // sorts before 'a' (97). A locale-aware comparator would flip this —
+  // verify we match the default so plain string columns are unaffected.
+  expect(htmlTextComparator('Z', 'a')).toBeLessThan(0);
+  expect(htmlTextComparator('a', 'Z')).toBeGreaterThan(0);
+  expect(htmlTextComparator('apple', 'banana')).toBeLessThan(0);
+});

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/htmlTextFilterValueGetter.ts

@@ -0,0 +1,74 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { isProbablyHTML, sanitizeHtml } from '@superset-ui/core';
+import { ValueGetterParams } from '@superset-ui/core/components/ThemedAgGridReact';
+
+const stripHtmlToText = (html: string): string => {
+  const doc = new DOMParser().parseFromString(sanitizeHtml(html), 'text/html');
+  return (doc.body.textContent || '').trim();
+};
+
+// Cache the comparator-ready form per raw string. Both the HTML-detection
+// step (`isProbablyHTML`, which itself invokes DOMParser for HTML-looking
+// values) and the extraction step (`stripHtmlToText`, also DOMParser) are
+// expensive; sort runs `O(n log n)` comparator calls against the same set
+// of cell values. Memoizing the combined detection + extraction means each
+// unique cell value pays the cost once per session. Module-level scope;
+// bounded by the count of unique string cell values seen.
+const comparableTextCache = new Map<string, string>();
+
+const toComparableText = (raw: string): string => {
+  const cached = comparableTextCache.get(raw);
+  if (cached !== undefined) return cached;
+  const normalized = isProbablyHTML(raw) ? stripHtmlToText(raw) : raw;
+  comparableTextCache.set(raw, normalized);
+  return normalized;
+};
+
+/**
+ * Returns the visible-text representation of an HTML cell value so AG Grid
+ * filters and sort operate on what the user sees, not the underlying markup.
+ * Pass-through for non-HTML values.
+ */
+const htmlTextFilterValueGetter = (params: ValueGetterParams) => {
+  const raw = params.data?.[params.colDef.field as string];
+  return typeof raw === 'string' ? toComparableText(raw) : raw;
+};
+
+/**
+ * Comparator that mirrors AG Grid's default string comparator (codepoint
+ * order, nulls first), but extracts visible text from HTML values first
+ * so HTML cells sort by their displayed label. Plain (non-HTML) values
+ * pass through unchanged, preserving default ordering — e.g. 'Z' still
+ * sorts before 'a' as it does under the default comparator.
+ */
+export const htmlTextComparator = (a: unknown, b: unknown): number => {
+  const toText = (v: unknown) =>
+    typeof v === 'string' ? toComparableText(v) : v;
+  const aT = toText(a);
+  const bT = toText(b);
+  if (aT == null && bT == null) return 0;
+  if (aT == null) return -1;
+  if (bT == null) return 1;
+  if (typeof aT === 'number' && typeof bT === 'number') return aT - bT;
+  if (aT === bT) return 0;
+  return aT < bT ? -1 : 1;
+};
+
+export default htmlTextFilterValueGetter;

superset-frontend/plugins/plugin-chart-ag-grid-table/src/utils/useColDefs.ts

@@ -32,6 +32,9 @@ import {
 } from '../types';
 import getCellClass from './getCellClass';
 import filterValueGetter from './filterValueGetter';
+import htmlTextFilterValueGetter, {
+  htmlTextComparator,
+} from './htmlTextFilterValueGetter';
 import dateFilterComparator from './dateFilterComparator';
 import DateWithFormatter from './DateWithFormatter';
 import { getAggFunc } from './getAggFunc';
@@ -317,6 +320,24 @@ export const useColDefs = ({
         ...(isPercentMetric && {
           filterValueGetter,
         }),
+        ...(dataType === GenericDataType.String &&
+          !serverPagination && {
+            // HTML cells (e.g. anchor markup) are rendered by TextCellRenderer
+            // via dangerouslySetInnerHTML; without these the filter and sort
+            // operate on raw HTML so the URL inside the markup dictates order
+            // and the "Contains" filter matches against the raw HTML string.
+            //
+            // Gated on !serverPagination: in server-pagination mode sort and
+            // filter are both delegated to the backend (which sees raw HTML
+            // in the database), so applying the visible-text getter only on
+            // the client would create a mismatch where the typed filter
+            // value is stripped client-side but the server query still
+            // operates on the raw HTML. Server-paginated tables with HTML
+            // columns are out of scope for this fix and would require
+            // server-side handling.
+            filterValueGetter: htmlTextFilterValueGetter,
+            comparator: htmlTextComparator,
+          }),
         ...(dataType === GenericDataType.Temporal && {
           // Use dateFilterValueGetter so AG Grid correctly identifies null dates for blank filter
           filterValueGetter: dateFilterValueGetter,

superset-frontend/plugins/plugin-chart-echarts/src/Sunburst/transformProps.ts

@@ -285,8 +285,6 @@ export default function transformProps(
   }
   const labelProps = {
     color: theme.colorText,
-    textBorderColor: theme.colorBgBase,
-    textBorderWidth: 1,
   };
   const traverse = (
     treeNodes: TreeNode[],

superset-frontend/plugins/plugin-chart-echarts/test/Sunburst/transformProps.test.ts

@@ -0,0 +1,53 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import { ChartProps } from '@superset-ui/core';
+import { supersetTheme } from '@apache-superset/core/theme';
+import { EchartsSunburstChartProps } from '../../src/Sunburst/types';
+import transformProps from '../../src/Sunburst/transformProps';
+
+const formData = {
+  colorScheme: 'bnbColors',
+  datasource: '3__table',
+  groupby: ['category'],
+  metric: 'sum__value',
+};
+
+const chartProps = new ChartProps({
+  formData,
+  width: 800,
+  height: 600,
+  queriesData: [
+    {
+      data: [
+        { category: 'A', sum__value: 10 },
+        { category: 'B', sum__value: 20 },
+      ],
+    },
+  ],
+  theme: supersetTheme,
+});
+
+test('series label has no textBorderColor or textBorderWidth', () => {
+  const { echartOptions } = transformProps(
+    chartProps as EchartsSunburstChartProps,
+  );
+  const series = (echartOptions as any).series[0];
+  expect(series.label).not.toHaveProperty('textBorderColor');
+  expect(series.label).not.toHaveProperty('textBorderWidth');
+});

superset-frontend/src/explore/components/ExploreContentPopover.tsx

@@ -22,12 +22,18 @@ export const ExplorePopoverContent = styled.div`
   .edit-popover-resize {
     transform: scaleX(-1);
     float: right;
-    margin-top: ${({ theme }) => theme.sizeUnit * 4}px;
-    margin-right: ${({ theme }) => theme.sizeUnit * -1}px;
+    margin-top: ${({ theme }) => theme.margin}px;
+    margin-right: ${({ theme }) => theme.marginXXS * -1}px;
     color: ${({ theme }) => theme.colorIcon};
     cursor: nwse-resize;
   }
   .filter-sql-editor {
     border: ${({ theme }) => theme.colorBorder} solid thin;
   }
+  && .ant-tabs-nav {
+    margin-bottom: ${({ theme }) => theme.marginSM}px;
+  }
+  && .ant-form-item {
+    margin-bottom: ${({ theme }) => theme.marginXS}px;
+  }
 `;

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopover/index.tsx

@@ -78,14 +78,6 @@ interface AdhocFilterEditPopoverState {
 }
 
 const FilterPopoverContentContainer = styled.div`
-  .adhoc-filter-edit-tabs > .nav-tabs {
-    margin-bottom: ${({ theme }) => theme.sizeUnit * 2}px;
-
-    & > li > a {
-      padding: ${({ theme }) => theme.sizeUnit}px;
-    }
-  }
-
   #filter-edit-popover {
     max-width: none;
   }
@@ -97,21 +89,17 @@ const FilterPopoverContentContainer = styled.div`
   .filter-edit-clause-section {
     display: flex;
     flex-direction: row;
-    gap: ${({ theme }) => theme.sizeUnit * 5}px;
-  }
-
-  .adhoc-filter-simple-column-dropdown {
-    margin-top: ${({ theme }) => theme.sizeUnit * 5}px;
+    gap: ${({ theme }) => theme.marginMD}px;
   }
 `;
 
 const FilterActionsContainer = styled.div`
-  margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
+  margin-top: ${({ theme }) => theme.marginXS}px;
 `;
 
 const LayerSelectContainer = styled.div`
-  margin-top: ${({ theme }) => theme.sizeUnit * 2}px;
-  margin-bottom: ${({ theme }) => theme.sizeUnit * 12}px;
+  margin-top: ${({ theme }) => theme.marginXS}px;
+  margin-bottom: ${({ theme }) => theme.marginXXL}px;
 `;
 
 export default class AdhocFilterEditPopover extends Component<

superset-frontend/src/explore/components/controls/FilterControl/AdhocFilterEditPopoverSimpleTabContent/index.tsx

@@ -523,8 +523,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
   const subjectComponent = (
     <Select
       css={{
-        marginTop: theme.sizeUnit * 4,
-        marginBottom: theme.sizeUnit * 4,
+        marginBottom: theme.marginXS,
       }}
       data-test="select-element"
       options={columns.map(column => ({
@@ -565,7 +564,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
         >
           <SelectWithLabel
             css={css`
-              margin-top: ${theme.sizeUnit * 4}px;
+              margin-top: ${theme.marginXS}px;
             `}
             labelText={labelText}
             options={suggestions}
@@ -581,7 +580,7 @@ const AdhocFilterEditPopoverSimpleTabContent: FC<Props> = props => {
         >
           <div
             css={css`
-              margin-top: ${theme.sizeUnit * 4}px;
+              margin-top: ${theme.marginXS}px;
             `}
           />
           <Input

superset-frontend/src/explore/components/controls/LayerConfigsControl/LayerConfigsPopoverContent.tsx

@@ -64,7 +64,6 @@ export const StyledCloseButton = styled(Button)`
     color: ${theme.colorPrimaryText};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
-    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;
@@ -113,7 +112,6 @@ export const StyledSaveButton = styled(Button)`
     color: ${theme.colorTextLightSolid};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
-    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;

superset-frontend/src/explore/components/controls/MapViewControl/MapViewControl.tsx

@@ -36,7 +36,6 @@ export const StyledExtentButton = styled(Button)`
     color: ${theme.colorPrimaryText};
     font-size: ${theme.fontSizeSM}px;
     font-weight: ${theme.fontWeightStrong};
-    text-transform: uppercase;
     min-width: ${theme.sizeUnit * 36};
     min-height: ${theme.sizeUnit * 8};
     box-shadow: none;

superset/app.py

@@ -71,22 +71,6 @@ def create_app(
             # value of app_root so things work out of the box
             if not app.config["STATIC_ASSETS_PREFIX"]:
                 app.config["STATIC_ASSETS_PREFIX"] = app_root
-            # Prefix APP_ICON path with subdirectory root for subdirectory deployments
-            if (
-                app.config.get("APP_ICON", "").startswith("/static/")
-                and app_root != "/"
-            ):
-                app.config["APP_ICON"] = f"{app_root}{app.config['APP_ICON']}"
-                # Also update theme tokens for subdirectory deployments
-                for theme_key in ("THEME_DEFAULT", "THEME_DARK"):
-                    theme = app.config[theme_key]
-                    token = theme.get("token", {})
-                    # Update brandLogoUrl if it points to /static/
-                    if token.get("brandLogoUrl", "").startswith("/static/"):
-                        token["brandLogoUrl"] = f"{app_root}{token['brandLogoUrl']}"
-                    # Update brandLogoHref if it's the default "/"
-                    if token.get("brandLogoHref") == "/":
-                        token["brandLogoHref"] = app_root
             if app.config["APPLICATION_ROOT"] == "/":
                 app.config["APPLICATION_ROOT"] = app_root
 

superset/mcp_service/auth.py

@@ -516,6 +516,47 @@ def _cleanup_session_on_error() -> None:
         logger.warning("Error cleaning up session after exception: %s", e)
 
 
+def _remove_session_safe() -> None:
+    """Remove the scoped SQLAlchemy session, tolerating SSL/connection errors.
+
+    Thread-pool workers reuse threads across requests.  Before each tool call
+    the session is removed to prevent a prior request's thread-local session
+    from leaking into the next one.  If the underlying DBAPI connection died
+    between requests (e.g. RDS SSL idle-timeout or max-connection-age), the
+    rollback implicit in ``session.close()`` raises a ``DBAPIError`` subclass
+    (``OperationalError`` for psycopg2, ``InterfaceError`` for some other
+    drivers).
+
+    When that happens:
+    1. Invalidate the dead connection so the pool discards it (rather than
+       returning a broken connection to the next caller).
+    2. Retry ``remove()`` to deregister the session from the scoped registry.
+
+    The tool call still proceeds because a fresh connection will be obtained
+    on the next DB access.
+    """
+    from sqlalchemy.exc import DBAPIError
+
+    from superset.extensions import db
+
+    try:
+        db.session.remove()
+    except DBAPIError as exc:
+        logger.warning(
+            "Connection error during pre-call session cleanup "
+            "(likely SSL/idle timeout); invalidating connection and retrying: %s",
+            exc,
+        )
+        try:
+            db.session.invalidate()
+        except Exception as invalidate_exc:
+            logger.debug(
+                "Could not invalidate session after connection error: %s",
+                invalidate_exc,
+            )
+        db.session.remove()  # retry: session deregisters cleanly after invalidation
+
+
 def mcp_auth_hook(tool_func: F) -> F:  # noqa: C901
     """
     Authentication and authorization decorator for MCP tools.
@@ -638,9 +679,7 @@ def mcp_auth_hook(tool_func: F) -> F:  # noqa: C901
                 # still be bound to a different tenant's DB engine. Removing it here
                 # ensures the next DB access creates a fresh session bound to the
                 # correct engine for the current request.
-                from superset.extensions import db
-
-                db.session.remove()
+                _remove_session_safe()
                 user = _setup_user_context()
 
                 # No Flask context - this is a FastMCP internal operation

superset/mcp_service/chart/tool/get_chart_data.py

@@ -46,7 +46,10 @@ from superset.mcp_service.chart.schemas import (
     GetChartDataRequest,
     PerformanceMetadata,
 )
-from superset.mcp_service.utils import sanitize_for_llm_context
+from superset.mcp_service.utils import (
+    escape_llm_context_delimiters,
+    sanitize_for_llm_context,
+)
 from superset.mcp_service.utils.cache_utils import get_cache_status_from_result
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
@@ -199,8 +202,12 @@ async def get_chart_data(  # noqa: C901
 
         if not chart:
             await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
+            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
             return ChartError(
-                error=f"No chart found with identifier: {request.identifier}",
+                error=(
+                    f"No chart found with identifier: {safe_id}."
+                    " Use list_charts to get valid chart IDs."
+                ),
                 error_type="NotFound",
             )
 

superset/mcp_service/chart/tool/get_chart_preview.py

@@ -47,7 +47,10 @@ from superset.mcp_service.chart.schemas import (
     URLPreview,
     VegaLitePreview,
 )
-from superset.mcp_service.utils import sanitize_for_llm_context
+from superset.mcp_service.utils import (
+    escape_llm_context_delimiters,
+    sanitize_for_llm_context,
+)
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
     OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -1201,8 +1204,22 @@ async def _get_chart_preview_internal(  # noqa: C901
 
         if not chart:
             await ctx.warning("Chart not found: identifier=%s" % (request.identifier,))
+            is_form_data_key = (
+                isinstance(request.identifier, str)
+                and len(request.identifier) > 8
+                and not request.identifier.isdigit()
+            )
+            if is_form_data_key:
+                recovery = (
+                    "If using a form_data_key, it may have expired — "
+                    "use generate_explore_link to get a fresh key, "
+                    "or use list_charts to find a saved chart by ID."
+                )
+            else:
+                recovery = "Use list_charts to get valid chart IDs."
+            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
             return ChartError(
-                error=f"No chart found with identifier: {request.identifier}",
+                error=f"No chart found with identifier: {safe_id}. {recovery}",
                 error_type="NotFound",
             )
 

superset/mcp_service/chart/tool/update_chart.py

@@ -47,6 +47,7 @@ from superset.mcp_service.chart.schemas import (
     PerformanceMetadata,
     UpdateChartRequest,
 )
+from superset.mcp_service.utils import escape_llm_context_delimiters
 from superset.mcp_service.utils.oauth2_utils import (
     build_oauth2_redirect_message,
     OAUTH2_CONFIG_ERROR_MESSAGE,
@@ -337,17 +338,18 @@ async def update_chart(  # noqa: C901
             chart = find_chart_by_identifier(request.identifier)
 
         if not chart:
+            safe_id = escape_llm_context_delimiters(str(request.identifier)[:200])
+            not_found_msg = (
+                f"No chart found with identifier: {safe_id}."
+                " Use list_charts to get valid chart IDs."
+            )
             return GenerateChartResponse.model_validate(
                 {
                     "chart": None,
                     "error": {
                         "error_type": "NotFound",
-                        "message": (
-                            f"No chart found with identifier: {request.identifier}"
-                        ),
-                        "details": (
-                            f"No chart found with identifier: {request.identifier}"
-                        ),
+                        "message": not_found_msg,
+                        "details": not_found_msg,
                     },
                     "success": False,
                     "schema_version": "2.0",

superset/mcp_service/dashboard/tool/add_chart_to_existing_dashboard.py

@@ -334,7 +334,10 @@ def _find_and_authorize_dashboard(
             dashboard=None,
             dashboard_url=None,
             position=None,
-            error=f"Dashboard with ID {dashboard_id} not found",
+            error=(
+                f"Dashboard with ID {dashboard_id} not found."
+                " Use list_dashboards to get valid dashboard IDs."
+            ),
         )
 
     try:
@@ -392,7 +395,10 @@ def add_chart_to_existing_dashboard(
                     dashboard=None,
                     dashboard_url=None,
                     position=None,
-                    error=f"Chart with ID {request.chart_id} not found",
+                    error=(
+                        f"Chart with ID {request.chart_id} not found."
+                        " Use list_charts to get valid chart IDs."
+                    ),
                 )
 
             # Validate dataset access for the chart.

superset/mcp_service/dashboard/tool/generate_dashboard.py

@@ -230,7 +230,10 @@ def generate_dashboard(  # noqa: C901
                 return GenerateDashboardResponse(
                     dashboard=None,
                     dashboard_url=None,
-                    error=f"Charts not found: {list(missing_chart_ids)}",
+                    error=(
+                        f"Charts not found: {sorted(missing_chart_ids)}."
+                        " Use list_charts to get valid chart IDs."
+                    ),
                 )
 
             # Validate dataset access for each chart.

superset/mcp_service/dataset/tool/query_dataset.py

@@ -183,7 +183,10 @@ async def query_dataset(  # noqa: C901
         if dataset is None:
             await ctx.error("Dataset not found: identifier=%s" % (request.dataset_id,))
             return DatasetError.create(
-                error=f"No dataset found with identifier: {request.dataset_id}",
+                error=(
+                    f"No dataset found with identifier: {request.dataset_id}."
+                    " Use list_datasets to get valid dataset IDs."
+                ),
                 error_type="NotFound",
             )
 

superset/mcp_service/sql_lab/tool/execute_sql.py

@@ -100,7 +100,10 @@ async def execute_sql(request: ExecuteSqlRequest, ctx: Context) -> ExecuteSqlRes
                 )
                 return ExecuteSqlResponse(
                     success=False,
-                    error=f"Database with ID {request.database_id} not found",
+                    error=(
+                        f"Database with ID {request.database_id} not found."
+                        " Use list_databases to get valid database IDs."
+                    ),
                     error_type=SupersetErrorType.DATABASE_NOT_FOUND_ERROR.value,
                 )
 

superset/mcp_service/sql_lab/tool/open_sql_lab_with_context.py

@@ -103,7 +103,8 @@ def open_sql_lab_with_context(
             database = DatabaseDAO.find_by_id(request.database_connection_id)
         if not database:
             error_message = (
-                f"Database with ID {request.database_connection_id} not found"
+                f"Database with ID {request.database_connection_id} not found."
+                " Use list_databases to get valid database IDs."
             )
             return _sanitize_sql_lab_response_for_llm_context(
                 SqlLabResponse(

superset/utils/encrypt.py

@@ -99,14 +99,29 @@ class SecretsMigrator:
 
     def discover_encrypted_fields(self) -> dict[str, dict[str, EncryptedType]]:
         """
-        Iterates over SqlAlchemy's metadata, looking for EncryptedType
-        columns along the way. Builds up a dict of
+        Iterates over ORM-mapped tables, looking for EncryptedType columns
+        along the way. Builds up a dict of
         table_name -> dict of col_name: enc type instance
-        :return:
+
+        Superset's ORM models inherit from Flask-AppBuilder's declarative base
+        (`flask_appbuilder.Model`), whose MetaData is distinct from
+        `db.metadata`. We combine both sources so encrypted columns are found
+        regardless of which base a model uses. FAB's metadata takes precedence
+        when a table name appears in both registries.
+
+        :return: mapping of table name to a dict of {column name: EncryptedType}
         """
+        from flask_appbuilder import (  # pylint: disable=import-outside-toplevel
+            Model as FABModel,
+        )
+
         meta_info: dict[str, Any] = {}
 
+        tables: dict[str, Any] = dict(FABModel.metadata.tables)
         for table_name, table in self._db.metadata.tables.items():
+            tables.setdefault(table_name, table)
+
+        for table_name, table in tables.items():
             for col_name, col in table.columns.items():
                 if isinstance(col.type, EncryptedType):
                     cols = meta_info.get(table_name, {})

superset/utils/oauth2.py

@@ -38,7 +38,7 @@ from superset.superset_typing import OAuth2ClientConfig, OAuth2State
 
 if TYPE_CHECKING:
     from superset.db_engine_specs.base import BaseEngineSpec
-    from superset.models.core import Database, DatabaseUserOAuth2Tokens
+    from superset.models.core import Database
 
 JWT_EXPIRATION = timedelta(minutes=5)
 
@@ -116,7 +116,7 @@ def get_oauth2_access_token(
         return token.access_token
 
     if token.refresh_token:
-        return refresh_oauth2_token(config, database_id, user_id, db_engine_spec, token)
+        return refresh_oauth2_token(config, database_id, user_id, db_engine_spec)
 
     # since the access token is expired and there's no refresh token, delete the entry
     db.session.delete(token)
@@ -129,8 +129,10 @@ def refresh_oauth2_token(
     database_id: int,
     user_id: int,
     db_engine_spec: type[BaseEngineSpec],
-    token: DatabaseUserOAuth2Tokens,
 ) -> str | None:
+    # pylint: disable=import-outside-toplevel
+    from superset.models.core import DatabaseUserOAuth2Tokens
+
     # Use longer TTL for OAuth2 token refresh (may involve network calls)
     with DistributedLock(
         namespace="refresh_oauth2_token",
@@ -138,6 +140,22 @@ def refresh_oauth2_token(
         user_id=user_id,
         database_id=database_id,
     ):
+        # Short circuit in case another request already deleted the token
+        token = (
+            db.session.query(DatabaseUserOAuth2Tokens)
+            .filter_by(user_id=user_id, database_id=database_id)
+            .one_or_none()
+        )
+        if token is None:
+            return None
+
+        if token.access_token and datetime.now() < token.access_token_expiration:
+            return token.access_token
+
+        if not token.refresh_token:
+            db.session.delete(token)
+            return None
+
         try:
             token_response = db_engine_spec.get_oauth2_fresh_token(
                 config,

tests/integration_tests/superset_test_config.py

@@ -78,15 +78,6 @@ FEATURE_FLAGS = {
 
 WEBDRIVER_BASEURL = "http://0.0.0.0:8081/"
 
-# Enable CORS for embedded dashboard E2E tests (test app on port 9000)
-ENABLE_CORS = True
-CORS_OPTIONS: dict = {
-    "origins": [
-        "http://localhost:9000",
-    ],
-    "supports_credentials": True,
-}
-
 
 def GET_FEATURE_FLAGS_FUNC(ff):  # noqa: N802
     ff_copy = copy(ff)
@@ -95,7 +86,6 @@ def GET_FEATURE_FLAGS_FUNC(ff):  # noqa: N802
 
 
 TESTING = True
-TALISMAN_ENABLED = False
 WTF_CSRF_ENABLED = False
 
 FAB_ROLES = {"TestRole": [["Security", "menu_access"], ["List Users", "menu_access"]]}

tests/integration_tests/utils/encrypt_tests.py

@@ -89,6 +89,21 @@ class EncryptedFieldTest(SupersetTestCase):
                         " encrypted_field_factory"
                     )
 
+    def test_discover_encrypted_fields_finds_dbs_table(self):
+        """
+        Ensure discover_encrypted_fields finds the encrypted columns on the
+        dbs table (password, encrypted_extra, server_cert). This guards
+        against db.metadata diverging from db.Model.metadata.
+        """
+        migrator = SecretsMigrator("")
+        encrypted_fields = migrator.discover_encrypted_fields()
+        assert "dbs" in encrypted_fields, (
+            "dbs table not found in encrypted fields — "
+            "discover_encrypted_fields may be using the wrong MetaData instance"
+        )
+        dbs_cols = set(encrypted_fields["dbs"].keys())
+        assert {"password", "encrypted_extra", "server_cert"}.issubset(dbs_cols)
+
     def test_lazy_key_resolution(self):
         """
         Verify that the encryption key is resolved lazily at runtime,

tests/unit_tests/mcp_service/sql_lab/tool/test_open_sql_lab_with_context.py

@@ -298,7 +298,8 @@ class TestOpenSqlLabWithContext:
                 field_path=("title",),
             )
             assert response.error == sanitize_for_llm_context(
-                "Database with ID 404 not found",
+                "Database with ID 404 not found."
+                " Use list_databases to get valid database IDs.",
                 field_path=("error",),
             )
         finally:

tests/unit_tests/mcp_service/test_auth_user_resolution.py

@@ -409,6 +409,50 @@ def test_mcp_auth_hook_removes_stale_db_session_in_sync_wrapper(app) -> None:
     assert result == "fresh"
 
 
+def test_sync_wrapper_handles_ssl_error_on_pre_call_remove(app) -> None:
+    """sync_wrapper tolerates OperationalError from db.session.remove() before the call.
+
+    If the underlying DBAPI connection died between requests (e.g. RDS SSL
+    idle-timeout), the rollback implicit in session.close() raises
+    OperationalError.  _remove_session_safe() should:
+    - Log a warning
+    - Call session.invalidate() to mark the dead connection for pool discard
+    - Retry session.remove() so the registry is clean
+    - Allow the tool to run successfully
+    """
+    from sqlalchemy.exc import OperationalError as SAOperationalError
+
+    fresh_user = _make_mock_user("fresh")
+
+    def dummy_tool() -> str:
+        """Dummy sync tool."""
+        return g.user.username
+
+    wrapped = mcp_auth_hook(dummy_tool)
+
+    with app.test_request_context():
+        g.user = fresh_user
+        with patch("superset.extensions.db") as mock_db:
+            mock_db.session.remove.side_effect = [
+                SAOperationalError(
+                    "SSL connection has been closed unexpectedly", None, None
+                ),
+                None,  # second call succeeds
+            ]
+
+            with patch(
+                "superset.mcp_service.auth.get_user_from_request",
+                return_value=fresh_user,
+            ):
+                result = wrapped()
+
+    assert result == "fresh"
+    assert mock_db.session.invalidate.called, "invalidate() must be called on SSL error"
+    assert mock_db.session.remove.call_count == 2, (
+        "remove() must be retried after SSL error"
+    )
+
+
 # -- default_user_resolver --
 
 

tests/unit_tests/utils/oauth2_tests.py

@@ -131,10 +131,12 @@ def test_refresh_oauth2_token_deletes_token_on_oauth2_exception(
         "Token revoked"
     )
     token = mocker.MagicMock()
+    token.access_token = None
     token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
 
     with pytest.raises(OAuth2ExceptionError):
-        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec, token)
+        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
 
     db.session.delete.assert_called_with(token)
     db.session.flush.assert_called_once()
@@ -160,10 +162,12 @@ def test_refresh_oauth2_token_keeps_token_on_other_exception(
     db_engine_spec.oauth2_exception = OAuth2ExceptionError
     db_engine_spec.get_oauth2_fresh_token.side_effect = Exception("Network error")
     token = mocker.MagicMock()
+    token.access_token = None
     token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
 
     with pytest.raises(Exception, match="Network error"):
-        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec, token)
+        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
 
     db.session.delete.assert_not_called()
 
@@ -176,16 +180,18 @@ def test_refresh_oauth2_token_no_access_token_in_response(
 
     This can happen when the refresh token was revoked.
     """
-    mocker.patch("superset.utils.oauth2.db")
+    db = mocker.patch("superset.utils.oauth2.db")
     mocker.patch("superset.utils.oauth2.DistributedLock")
     db_engine_spec = mocker.MagicMock()
     db_engine_spec.get_oauth2_fresh_token.return_value = {
         "error": "invalid_grant",
     }
     token = mocker.MagicMock()
+    token.access_token = None
     token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
 
-    result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec, token)
+    result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
 
     assert result is None
 
@@ -208,10 +214,12 @@ def test_refresh_oauth2_token_updates_refresh_token(
         "refresh_token": "new-refresh-token",
     }
     token = mocker.MagicMock()
+    token.access_token = None
     token.refresh_token = "old-refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
 
     with freeze_time("2024-01-01"):
-        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec, token)
+        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
 
     assert token.access_token == "new-access-token"  # noqa: S105
     assert token.access_token_expiration == datetime(2024, 1, 1, 1)
@@ -236,16 +244,127 @@ def test_refresh_oauth2_token_keeps_refresh_token(
         "expires_in": 3600,
     }
     token = mocker.MagicMock()
+    token.access_token = None
     token.refresh_token = "original-refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
 
     with freeze_time("2024-01-01"):
-        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec, token)
+        refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
 
     assert token.access_token == "new-access-token"  # noqa: S105
     assert token.refresh_token == "original-refresh-token"  # noqa: S105
     db.session.add.assert_called_with(token)
 
 
+def test_refresh_oauth2_token_refreshes_when_access_token_expired_under_lock(
+    mocker: MockerFixture,
+) -> None:
+    """
+    Test that refresh_oauth2_token triggers a refresh when the access_token is expired.
+
+    When the re-query under the lock returns a token whose access_token has expired
+    but a refresh_token is available, the function should call the token endpoint
+    and persist the new access_token.
+    """
+    db = mocker.patch("superset.utils.oauth2.db")
+    mocker.patch("superset.utils.oauth2.DistributedLock")
+    db_engine_spec = mocker.MagicMock()
+    db_engine_spec.get_oauth2_fresh_token.return_value = {
+        "access_token": "new-access-token",
+        "expires_in": 3600,
+    }
+    token = mocker.MagicMock()
+    token.access_token = "expired-token"  # noqa: S105
+    token.access_token_expiration = datetime(2024, 1, 1)
+    token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
+
+    with freeze_time("2024-01-02"):
+        result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
+
+    assert result == "new-access-token"
+    db_engine_spec.get_oauth2_fresh_token.assert_called_once_with(
+        DUMMY_OAUTH2_CONFIG, "refresh-token"
+    )
+    db.session.add.assert_called_with(token)
+
+
+def test_refresh_oauth2_token_returns_existing_token_when_still_valid_under_lock(
+    mocker: MockerFixture,
+) -> None:
+    """
+    Test that refresh_oauth2_token returns the existing access_token if still valid.
+
+    When concurrent requests are triggered and the first one refreshes the token and
+    releases the lock before the second one gets to `refresh_oauth2_token`, the second
+    request should pick up the already-refreshed access_token instead of refreshing
+    it again.
+    """
+    db = mocker.patch("superset.utils.oauth2.db")
+    mocker.patch("superset.utils.oauth2.DistributedLock")
+    db_engine_spec = mocker.MagicMock()
+    token = mocker.MagicMock()
+    token.access_token = "fresh-access-token"  # noqa: S105
+    token.access_token_expiration = datetime(2024, 1, 2)
+    token.refresh_token = "refresh-token"  # noqa: S105
+    db.session.query().filter_by().one_or_none.return_value = token
+
+    with freeze_time("2024-01-01"):
+        result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
+
+    assert result == "fresh-access-token"
+    db_engine_spec.get_oauth2_fresh_token.assert_not_called()
+    db.session.delete.assert_not_called()
+
+
+def test_refresh_oauth2_token_deletes_when_no_refresh_token_under_lock(
+    mocker: MockerFixture,
+) -> None:
+    """
+    Test that refresh_oauth2_token deletes the row when there's no refresh_token.
+
+    When the token has expired and the re-query under the lock shows no refresh_token
+    is available, the row should be deleted and None returned so the caller can
+    trigger the OAuth2 dance.
+    """
+    db = mocker.patch("superset.utils.oauth2.db")
+    mocker.patch("superset.utils.oauth2.DistributedLock")
+    db_engine_spec = mocker.MagicMock()
+    token = mocker.MagicMock()
+    token.access_token = "expired-token"  # noqa: S105
+    token.access_token_expiration = datetime(2024, 1, 1)
+    token.refresh_token = None
+    db.session.query().filter_by().one_or_none.return_value = token
+
+    with freeze_time("2024-01-02"):
+        result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
+
+    assert result is None
+    db.session.delete.assert_called_with(token)
+    db_engine_spec.get_oauth2_fresh_token.assert_not_called()
+
+
+def test_refresh_oauth2_token_returns_none_when_row_deleted_under_lock(
+    mocker: MockerFixture,
+) -> None:
+    """
+    Test that refresh_oauth2_token returns None when the row is gone under the lock.
+
+    When concurrent requests are triggered and the first one deletes the token row and
+    releases the lock before the second one gets to `refresh_oauth2_token`, the token
+    is queried again to avoid a stale reference.
+    """
+    db = mocker.patch("superset.utils.oauth2.db")
+    mocker.patch("superset.utils.oauth2.DistributedLock")
+    db_engine_spec = mocker.MagicMock()
+    db.session.query().filter_by().one_or_none.return_value = None
+
+    result = refresh_oauth2_token(DUMMY_OAUTH2_CONFIG, 1, 1, db_engine_spec)
+
+    assert result is None
+    db_engine_spec.get_oauth2_fresh_token.assert_not_called()
+
+
 def test_generate_code_verifier_length() -> None:
     """
     Test that generate_code_verifier produces a string of valid length (RFC 7636).