From 79a953ab669078b5ad36c8c3893460f4f6161e1d Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 27 Oct 2026 07:08:10 -0400 Subject: [PATCH 1/2] =?UTF-8?q?=F0=9F=A7=AA=20fix:=20Delete=20the=20USER?= =?UTF-8?q?=20Profile=20the=20Config=20Shell=20Spec=20Creates=20and=20Unbr?= =?UTF-8?q?eak=20the=20Live=20Profiles=20and=20Import=20Specs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit e2e/config-shell.spec.ts (live) created the USER role's profile on a clean database to test Back from a profile and never deleted it, so later runs and other specs started from a different state. The test now records when it created the profile, and an afterEach deletes it through the scope selector, as an admin would, while it still has no overrides. A profile the test found is left alone. The selector helper waits for the profile list to load before deciding whether USER has a profile, so that decision is never made on an empty, still-loading list. After confirming the delete, the cleanup waits for the profile list to come back before checking that USER is gone: the confirm view lists no options and stays up until the delete succeeds, so checking only that the row is gone would pass at once, even when the delete failed. e2e/profiles.spec.ts: the Registration section of a user profile shows two notes, the scope gate and the read-only note for the settings LibreChat sets at startup, so `#section-registration [role=note]` was a strict-mode violation. The locator now targets the scope note (`data-read-only=scope`). e2e/import.spec.ts: ensureUserProfile counted the "Create profile" button once after networkidle, before the roles query had rendered it, so on a fresh database it skipped creating the USER profile and "imports into an existing profile" timed out. It now waits until the page shows either the open profile or the Create profile button, and creates the profile in the second case. --- e2e/config-shell.spec.ts | 43 ++++++++++++++++++++++++++++++++++------ e2e/import.spec.ts | 7 ++++--- e2e/profiles.spec.ts | 6 +++--- 3 files changed, 44 insertions(+), 12 deletions(-) diff --git a/e2e/config-shell.spec.ts b/e2e/config-shell.spec.ts index acb90d4e..77d809a6 100644 --- a/e2e/config-shell.spec.ts +++ b/e2e/config-shell.spec.ts @@ -180,23 +180,54 @@ test.describe('Configuration shell: search and deep links', () => { }); test.describe('Configuration shell: profile scope and guard', () => { + /** Set once a test creates the USER role's profile, so cleanup never deletes one it found. */ + let createdUserProfile = false; + + /** Opens the scope selector once its profile list has loaded. */ async function openScopeSelector(page: Page) { await page.getByRole('button', { name: /^Select scope/ }).click(); - return page.getByRole('dialog'); + const dialog = page.getByRole('dialog'); + await expect(dialog.getByRole('option', { name: /^Base configuration/ })).toBeVisible(); + return dialog; } /** Opens the USER role's profile; on a clean database, creating it opens it. */ async function selectUserRole(page: Page) { const dialog = await openScopeSelector(page); - const user = dialog.getByText('USER', { exact: true }); - if ((await user.count()) === 0) { - await dialog.getByRole('button', { name: 'Create' }).click(); - await dialog.getByText('USER', { exact: true }).first().click(); + const user = dialog.getByRole('option', { name: /^USER / }); + if ((await user.count()) > 0) { + await user.click(); return; } - await user.first().click(); + await dialog.getByRole('button', { name: 'Create' }).click(); + await dialog.getByRole('option', { name: 'USER', exact: true }).click(); + createdUserProfile = true; } + /** + * Deletes the USER role's profile unless something has saved overrides to it since. + * The confirm view lists no options and stays up until the delete succeeds, so the + * profile list coming back is what proves the delete finished. + */ + async function deleteUserProfile(page: Page) { + await openTab(page, 'system'); + const dialog = await openScopeSelector(page); + const user = dialog.getByRole('option', { name: /^USER / }); + if ((await user.filter({ hasText: 'No overrides' }).count()) === 0) return; + await dialog.getByRole('option', { name: 'Delete profile USER', exact: true }).click(); + await dialog.getByRole('button', { name: 'Delete configuration' }).click(); + await expect(dialog.getByRole('option', { name: /^Base configuration/ })).toBeVisible({ + timeout: 10_000, + }); + await expect(user).toHaveCount(0); + } + + test.afterEach(async ({ page }) => { + if (!createdUserProfile) return; + createdUserProfile = false; + await deleteUserProfile(page); + }); + test('Back after selecting a profile restores Base in the URL and the UI', async ({ page }) => { await openTab(page, 'system'); await selectUserRole(page); diff --git a/e2e/import.spec.ts b/e2e/import.spec.ts index 505ca46a..1d9b3cd8 100644 --- a/e2e/import.spec.ts +++ b/e2e/import.spec.ts @@ -31,11 +31,12 @@ async function previewAndApply(page: Page, target: RegExp) { /** Opens the USER role's profile, creating it on a clean database. */ async function ensureUserProfile(page: Page) { await page.goto('/configuration?tab=system&scope=role:USER'); - await page.waitForLoadState('networkidle'); + const opened = page.getByRole('button', { name: /^Select scope: USER/ }); const create = page.getByRole('button', { name: 'Create profile' }); - if ((await create.count()) === 0) return; + await expect(opened.or(create)).toBeVisible({ timeout: 30_000 }); + if (await opened.isVisible()) return; await create.click(); - await expect(page.getByRole('button', { name: /^Select scope: USER/ })).toBeVisible(); + await expect(opened).toBeVisible(); } async function startBalanceIn(page: Page, scope?: string) { diff --git a/e2e/profiles.spec.ts b/e2e/profiles.spec.ts index bfe80123..4de5d573 100644 --- a/e2e/profiles.spec.ts +++ b/e2e/profiles.spec.ts @@ -48,9 +48,9 @@ test.describe('Configuration: profile lifecycle', () => { await expect(page.getByRole('option', { name: new RegExp(`^${name}`) })).toBeVisible(); await page.keyboard.press('Escape'); - await expect(page.locator('#section-registration [role=note]')).toContainText( - 'applies to everyone', - ); + await expect( + page.locator('#section-registration [role=note][data-read-only=scope]'), + ).toContainText('applies to everyone'); await expect(page.locator('#section-registration input:not([disabled])')).toHaveCount(0); const startBalance = page.locator('#balance-startBalance'); From 74fe97ce581af08fe6315dc72b88c553423a311b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Tue, 27 Oct 2026 08:16:19 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=F0=9F=94=92=20fix:=20Run=20the=20Live=20Sp?= =?UTF-8?q?ecs=20That=20Share=20the=20USER=20Role=20Profile=20One=20at=20a?= =?UTF-8?q?=20Time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- e2e/config-shell.spec.ts | 8 ++- e2e/import.spec.ts | 32 ++++++---- e2e/live.ts | 133 ++++++++++++++++++++++++++++++++++++++- e2e/specs.spec.ts | 32 +++++----- 4 files changed, 173 insertions(+), 32 deletions(-) diff --git a/e2e/config-shell.spec.ts b/e2e/config-shell.spec.ts index 77d809a6..b2d18b16 100644 --- a/e2e/config-shell.spec.ts +++ b/e2e/config-shell.spec.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; import type { Page, Route } from '@playwright/test'; -import { LIVE, LIVE_SKIP_REASON } from './live'; +import { LIVE, LIVE_SKIP_REASON, lockUserProfile } from './live'; /** * Configuration page shell (WS5): loading and error routes, Expand all, @@ -179,7 +179,7 @@ test.describe('Configuration shell: search and deep links', () => { }); }); -test.describe('Configuration shell: profile scope and guard', () => { +test.describe('Configuration shell: Back from the USER role profile', () => { /** Set once a test creates the USER role's profile, so cleanup never deletes one it found. */ let createdUserProfile = false; @@ -222,7 +222,7 @@ test.describe('Configuration shell: profile scope and guard', () => { await expect(user).toHaveCount(0); } - test.afterEach(async ({ page }) => { + lockUserProfile(async (page) => { if (!createdUserProfile) return; createdUserProfile = false; await deleteUserProfile(page); @@ -237,7 +237,9 @@ test.describe('Configuration shell: profile scope and guard', () => { await expect(page).not.toHaveURL(/scope=/); await expect(page.getByRole('button', { name: /^Select scope: Base/ })).toBeVisible(); }); +}); +test.describe('Configuration shell: profile scope and guard', () => { test('a fresh visit opens Base, an unknown profile says so', async ({ page }) => { await openTab(page, 'system'); await expect(page.getByRole('button', { name: /^Select scope: Base/ })).toBeVisible(); diff --git a/e2e/import.spec.ts b/e2e/import.spec.ts index 1d9b3cd8..7d03c522 100644 --- a/e2e/import.spec.ts +++ b/e2e/import.spec.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; import type { Page } from '@playwright/test'; -import { LIVE, LIVE_SKIP_REASON } from './live'; +import { LIVE, LIVE_SKIP_REASON, lockUserProfile } from './live'; /** * The YAML import wizard needs LibreChat's admin config API, which the mock @@ -63,18 +63,24 @@ test('imports a snippet into Base, previewing exactly its keys', async ({ page } await expect(await startBalanceIn(page)).toHaveValue(value.toLocaleString('en-US')); }); -test('imports into an existing profile, comparing with its own values', async ({ page }) => { - const value = 200000 + (Date.now() % 100000); - await ensureUserProfile(page); - await openWizard(page, `balance:\n startBalance: ${value}\n`, 'role:USER'); - await expect(page.getByRole('radio', { name: /This profile \(USER\)/ })).toHaveAttribute( - 'aria-checked', - 'true', - ); - const { apply } = await previewAndApply(page, /into Role: USER$/); - await apply.click(); - await expect(page.getByText(/Imported 1 value into Role: USER/).first()).toBeVisible(); - await expect(await startBalanceIn(page, 'role:USER')).toHaveValue(value.toLocaleString('en-US')); +test.describe('the USER role profile', () => { + lockUserProfile(); + + test('imports into an existing profile, comparing with its own values', async ({ page }) => { + const value = 200000 + (Date.now() % 100000); + await ensureUserProfile(page); + await openWizard(page, `balance:\n startBalance: ${value}\n`, 'role:USER'); + await expect(page.getByRole('radio', { name: /This profile \(USER\)/ })).toHaveAttribute( + 'aria-checked', + 'true', + ); + const { apply } = await previewAndApply(page, /into Role: USER$/); + await apply.click(); + await expect(page.getByText(/Imported 1 value into Role: USER/).first()).toBeVisible(); + await expect(await startBalanceIn(page, 'role:USER')).toHaveValue( + value.toLocaleString('en-US'), + ); + }); }); test('imports into a role that has no profile yet', async ({ page }) => { diff --git a/e2e/live.ts b/e2e/live.ts index 46489f99..da6597d2 100644 --- a/e2e/live.ts +++ b/e2e/live.ts @@ -1,5 +1,8 @@ -import { expect } from '@playwright/test'; -import type { Page } from '@playwright/test'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import { test, expect } from '@playwright/test'; +import { mkdirSync, readFileSync, renameSync, rmSync, statSync, writeFileSync } from 'node:fs'; +import type { Page, TestInfo } from '@playwright/test'; /** * Configuration specs need a LibreChat backend that serves the admin config @@ -27,3 +30,129 @@ export async function openConfigTab(page: Page, tab: string) { await expect(form).toBeVisible({ timeout: 30_000 }); await page.waitForLoadState('networkidle'); } + +const LOOPBACK_HOSTS: ReadonlySet = new Set(['127.0.0.1', '[::1]']); + +/** One key per panel, however E2E_BASE_URL spells its loopback host or trailing slash. */ +function lockKey(baseUrl: string): string { + const url = new URL(baseUrl); + const host = LOOPBACK_HOSTS.has(url.hostname) ? 'localhost' : url.hostname; + const path = url.pathname.replace(/\/+$/, ''); + return `${host}-${url.port}${path}`.replace(/[^a-z0-9]+/gi, '-'); +} + +const LOCK_KEY = lockKey(process.env.E2E_BASE_URL ?? 'http://localhost:3000'); +const LOCK_DIR = join(tmpdir(), `librechat-admin-e2e-user-profile-${LOCK_KEY}`); +const LOCK_POLL_MS = 250; +/** Longer than a locked test holds the lock, unless its body and its cleanup both hang. */ +const LOCK_STALE_MS = 5 * 60_000; +/** Past the stale age, so a waiter takes over a hung holder's lock before it gives up. */ +const LOCK_WAIT_MS = LOCK_STALE_MS + 60_000; + +let lockHeld = false; + +function lockOwner(dir: string): number | undefined { + try { + const pid = Number(readFileSync(join(dir, 'owner'), 'utf8')); + return Number.isInteger(pid) && pid > 0 ? pid : undefined; + } catch { + return undefined; + } +} + +function isAlive(pid: number): boolean { + try { + process.kill(pid, 0); + return true; + } catch (error) { + return (error as NodeJS.ErrnoException).code === 'EPERM'; + } +} + +/** + * A lock is stale when its owner has exited, is this worker (one test runs at a time per + * worker, so it leaked), or has held it past the stale age. A lock whose owner file is not + * written yet is judged by age alone. + */ +function isStale(dir: string): boolean { + const pid = lockOwner(dir); + if (pid != null && (pid === process.pid || !isAlive(pid))) return true; + try { + return Date.now() - statSync(dir).mtimeMs > LOCK_STALE_MS; + } catch { + return false; + } +} + +/** + * Moves a stale lock aside and deletes it. When the move fails, another waiter got there + * first; when the lock moved aside is not stale, a waiter re-took it in between, so it goes back. + */ +function takeOver() { + const aside = `${LOCK_DIR}.stale-${process.pid}-${Date.now()}`; + try { + renameSync(LOCK_DIR, aside); + if (!isStale(aside)) renameSync(aside, LOCK_DIR); + } catch { + /* Another waiter moved it first, or took the lock while it was aside. */ + } finally { + rmSync(aside, { recursive: true, force: true }); + } +} + +/** One attempt: `mkdir` is atomic and fails while another worker holds the lock. */ +function tryLock(): boolean { + try { + mkdirSync(LOCK_DIR); + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== 'EEXIST') throw error; + if (isStale(LOCK_DIR)) takeOver(); + return false; + } + writeFileSync(join(LOCK_DIR, 'owner'), String(process.pid)); + return true; +} + +function releaseLock() { + if (!lockHeld) return; + lockHeld = false; + process.off('exit', releaseLock); + if (lockOwner(LOCK_DIR) === process.pid) rmSync(LOCK_DIR, { recursive: true, force: true }); +} + +/** Waits for the lock without spending the test's own timeout on the wait. */ +async function acquireLock(testInfo: TestInfo) { + const started = Date.now(); + const timeout = testInfo.timeout; + if (timeout) testInfo.setTimeout(timeout + LOCK_WAIT_MS); + while (!tryLock()) { + if (Date.now() - started > LOCK_WAIT_MS) { + const holder = lockOwner(LOCK_DIR) ?? 'unknown'; + throw new Error( + `Gave up after ${LOCK_WAIT_MS / 1000} s waiting for the USER role profile lock ` + + `${LOCK_DIR}, held by pid ${holder}`, + ); + } + await new Promise((resolve) => setTimeout(resolve, LOCK_POLL_MS)); + } + lockHeld = true; + process.on('exit', releaseLock); + if (timeout) testInfo.setTimeout(timeout + Date.now() - started); +} + +/** + * Runs each test of the calling describe alone against E2E_BASE_URL's backend, across workers + * and spec files. Every test that creates, uses or deletes the USER role's profile calls it: + * there are only two built-in roles, so no test gets a profile of its own. `cleanup` runs + * before the lock is released, also after a failure. + */ +export function lockUserProfile(cleanup?: (page: Page) => Promise) { + test.beforeEach(() => acquireLock(test.info())); + test.afterEach(async ({ page }) => { + try { + await cleanup?.(page); + } finally { + releaseLock(); + } + }); +} diff --git a/e2e/specs.spec.ts b/e2e/specs.spec.ts index 71b9d2b8..b852e6bc 100644 --- a/e2e/specs.spec.ts +++ b/e2e/specs.spec.ts @@ -1,6 +1,6 @@ import { test, expect } from '@playwright/test'; import type { Locator, Page } from '@playwright/test'; -import { LIVE, LIVE_SKIP_REASON } from './live'; +import { LIVE, LIVE_SKIP_REASON, lockUserProfile } from './live'; /** * Model specs journeys (WS11, the audit's specs journeys). They need a live @@ -176,19 +176,23 @@ test.describe('Model specs', () => { await expect(unsavedBar(page)).toHaveCount(0); }); - test('a profile shows the specs it inherits read-only until it customizes the list', async ({ - page, - }) => { - await openSpecs(page, '&scope=role:USER'); - const customize = page.getByRole('button', { name: /^Override for Role: USER/ }); - test.skip((await customize.count()) === 0, 'Needs a USER role profile without its own list'); - await expect(page.getByRole('button', { name: 'Add spec' })).toHaveCount(0); - await expect(cards(page).first()).toBeVisible(); - await customize.click(); - await expect(page.getByRole('button', { name: 'Add spec' })).toBeVisible(); - await expect(page.getByText(/replaces Base's list/)).toBeVisible(); - await page.getByRole('button', { name: 'Cancel' }).click(); - await expect(page.getByRole('button', { name: 'Add spec' })).toHaveCount(0); + test.describe('the USER role profile', () => { + lockUserProfile(); + + test('a profile shows the specs it inherits read-only until it customizes the list', async ({ + page, + }) => { + await openSpecs(page, '&scope=role:USER'); + const customize = page.getByRole('button', { name: /^Override for Role: USER/ }); + test.skip((await customize.count()) === 0, 'Needs a USER role profile without its own list'); + await expect(page.getByRole('button', { name: 'Add spec' })).toHaveCount(0); + await expect(cards(page).first()).toBeVisible(); + await customize.click(); + await expect(page.getByRole('button', { name: 'Add spec' })).toBeVisible(); + await expect(page.getByText(/replaces Base's list/)).toBeVisible(); + await page.getByRole('button', { name: 'Cancel' }).click(); + await expect(page.getByRole('button', { name: 'Add spec' })).toHaveCount(0); + }); }); test('the tab filter finds spec fields and phone width has no sideways scroll', async ({