Skip to content

🧪 fix: Delete the USER Profile the Config Shell Spec Creates and Unbreak the Live Profiles and Import Specs - #201

Merged
danny-avila merged 3 commits into
mainfrom
pr/fu-e2e-spec-hygiene
Oct 28, 2026
Merged

danny-avila merged 3 commits into
mainfrom
pr/fu-e2e-spec-hygiene

Conversation

@danny-avila

@danny-avila danny-avila commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

Pull Request Template

⚠️ Before Submitting a PR, Please Review:

  • Please ensure that you have thoroughly read and understood the Contributing Docs before submitting your Pull Request.

⚠️ Documentation Updates Notice:

  • Kindly note that documentation updates are managed in this repository: librechat.ai

Summary

Three live e2e specs (E2E_LIVE=1) left data behind or failed on main. The specs that use the USER role's profile now also take turns, so that the new cleanup can't delete that profile while another spec is using it. This PR changes only files in e2e/.

e2e/config-shell.spec.ts left a USER role profile behind. On a clean database, "Back after selecting a profile restores Base" creates the USER role's profile so it has a profile to select, and nothing deleted it. After one run, every later run and every other spec started from a different database. For example, import.spec.ts would then pass "imports into an existing profile" because the profile was already there.

  • The test now records whether it created the profile. If it did, an afterEach deletes it through the scope selector (Delete profile, then Delete configuration), the same way an admin would. The afterEach also runs when the test fails.
  • The cleanup waits for the delete to finish, and a delete that fails makes the test fail. After Delete configuration, the selector shows a confirm view that has no profile options and stays up until the delete succeeds. So the cleanup first waits for the profile list to come back, then checks that USER is gone. Checking only for the USER row would pass at once, even if the delete failed or the page closed before it was sent.
  • It deletes only a profile it created, and only while that profile still has no overrides. A USER profile that existed before the run is never touched, and neither is one that something else has saved values to in the meantime.
  • The selector helper now waits for the profile list to load before it checks whether USER already has a profile. That check decides whether the test created the profile, so it must never run against a list that is still loading.

The specs that use the USER role's profile now take turns. The cleanup above made a race possible. playwright.config.ts sets fullyParallel, and local runs use several workers. So import.spec.ts could open the empty USER profile that config-shell.spec.ts had just created, and the cleanup then deleted it while the import wizard was still using it. The import could also save its value between the "No overrides" check and the delete, and the delete then removed the value along with the profile. An override count can't tell which test owns the profile. LibreChat has only two built-in roles, ADMIN and USER, and import.spec.ts writes to both, so moving config-shell.spec.ts to the other role would only move the race.

  • e2e/live.ts adds lockUserProfile(cleanup?). Every test in a describe that calls it holds one shared lock for the whole test, across workers and spec files. A beforeEach waits for the lock. The afterEach runs the cleanup first and then releases the lock, also when the test failed or timed out. A process exit handler releases it as well.
  • Every test that creates, uses or deletes the USER role's profile takes the lock: "Back after selecting a profile" in config-shell.spec.ts, with the profile delete as its cleanup; "imports into an existing profile" in import.spec.ts; and "a profile shows the specs it inherits" in specs.spec.ts. "Back after selecting a profile" now has its own describe, so the other two profile-scope tests don't wait for the lock.
  • The lock is a directory created with mkdir, which is atomic, in the OS temp directory. Its name comes from E2E_BASE_URL's host, port and path, with 127.0.0.1, [::1] and a trailing slash read as the same panel, so runs against different panels never wait on each other and two spellings of one panel's URL still share the lock. The holder writes its process id into it.
  • A waiting test checks every 250 ms. The wait doesn't count toward the test's own timeout. After 6 minutes the test fails with an error that names the lock directory and the process id that holds it.
  • A lock whose holder process has exited, for example a killed worker, is taken over at once. So is a lock older than 5 minutes, which a locked test only reaches when its body and its cleanup both hang.
  • The cleanup rule doesn't change. It deletes only a profile the test created, and only while that profile has no overrides. With the lock held, no other test can save to the profile between that check and the delete.

e2e/profiles.spec.ts hit a strict-mode error. In a user profile, the Registration section now shows two notes: the scope gate ("Registration: applies to everyone") and the read-only note for settings LibreChat sets at startup. #section-registration [role=note] therefore matched both elements. The locator now targets the scope note: [role=note][data-read-only=scope].

e2e/import.spec.ts raced the roles query. ensureUserProfile counted the "Create profile" button once, right after networkidle. On a fresh database the button only appears once the roles query has returned, which can be a few seconds later. So the helper often found no button, skipped creating the profile, and "imports into an existing profile" timed out waiting for Import YAML. The helper now waits until the page shows either the open profile or the Create profile button, and clicks Create only in the second case.

Not in this PR

  • import.spec.ts still leaves its imports in the database: a Base balance.startBalance, values in the USER and ADMIN profiles, and one new Imported <timestamp> role each run. Its tests pick fresh values, so it re-runs fine anyway.
  • If profiles.spec.ts fails before its delete step, the profile for E2E_PROFILE_USER stays behind. A re-run then can't offer that user under Create.
  • specs.spec.ts "a profile shows the specs it inherits" skips without a USER role profile. Before this PR it usually had one, left behind by config-shell.spec.ts. On a clean database it now skips unless import.spec.ts has run first.
  • specs.spec.ts "a new spec asks for Endpoint and Model up front" fails even when it runs alone on a clean database: after Add spec, the new card's Endpoint select is not focused. This PR changes neither that test nor the panel, so main fails the same way. The rest of that serial describe then doesn't run, so the race runs below include only the USER profile test from specs.spec.ts.

Change Type

  • Bug fix (non-breaking change which fixes an issue)

Testing

All runs were live (E2E_LIVE=1) against a local LibreChat backend on a fresh database, with the worker counts given below. Between runs, config overrides and profiles were reset where noted.

main

Spec Starting data Result
config-shell.spec.ts clean 22 passed. The run leaves a role / USER config document with no overrides.
profiles.spec.ts clean Failed with a strict-mode violation: #section-registration [role=note] resolved to 2 elements, data-read-only="scope" and data-read-only="yamlOnly".
import.spec.ts clean, run twice Both runs failed the same way. "imports into an existing profile" timed out waiting for Import YAML, and the page showed "Role: USER has no profile" with its Create profile button. 2 passed, 3 did not run.

This branch

Spec Starting data Result
config-shell.spec.ts, twice in a row clean 22 passed both times. No config documents were left after either run.
config-shell.spec.ts an empty USER profile created first 22 passed. The same profile document (same _id) was still there afterwards.
config-shell.spec.ts, with a deliberate failure added right after the profile is created (not committed) clean The test failed as intended, and the afterEach still deleted the profile.
config-shell.spec.ts, with the delete request aborted in the browser (not committed) clean The afterEach failed: the profile list never came back within 10 s. The profile was left behind, and the run reported it. A cleanup that only checked for the USER row passed in the same setup and left the same profile.
profiles.spec.ts, twice in a row clean 2 passed both times, and no profile was left.
import.spec.ts, twice in a row clean 6 passed both times.
All three specs in one run, twice in a row clean 28 passed both times.
All three specs against the mock backend (no E2E_LIVE) n/a 27 skipped, setup passed.

The USER profile race (--workers=4, starting from a clean database each time; a change stream on the config collection and a watcher on the lock directory recorded what happened)

Run Result
Before the lock: "Back after selecting a profile", "imports into an existing profile" and "a profile shows the specs it inherits", --repeat-each=3, three runs All three runs failed, with 3 failures each. "imports into an existing profile" failed 8 times. In 4 of them the profile was gone by the final check, which found no Starting credits field (Not an input element). In 3 the profile was gone before the wizard opened, and the test timed out waiting for Import YAML. In 1 the import never reported success. In every run, the "Back" cleanup deleted the USER profile about 0.4 s after an import had saved balance.startBalance to it. One "Back" cleanup failed because another copy had already created USER again. Two of the three runs ended with no USER profile at all.
With the lock, the same three tests and flags 9 passed. The lock went from one test to the next, one holder at a time.
With the lock: config-shell.spec.ts, import.spec.ts and the specs.spec.ts USER profile test, --repeat-each=3, three runs 81 passed and 1 skipped each time (setup included). The skipped test is the specs test that ran before any USER profile existed. In each run, the first "Back" test created and deleted the USER profile while it held the lock, and the import created its own profile after that. Each run left only the data import.spec.ts is known to leave (see Not in this PR). No empty USER profile and no lock directory were left.
With the lock: config-shell.spec.ts and the specs test, --repeat-each=3 64 passed and 3 skipped. The three "Back" copies each created and deleted the USER profile in turn, and no config documents were left.
A worker killed with SIGKILL just after it took the lock That test failed with "worker process exited unexpectedly". The lock directory was left behind, naming the dead process. The next run of the three USER profile tests took the lock over at once: 4 passed in 7.2 s, and no lock directory was left.
A lock directory 6 minutes old, owned by a live process that is not a test Taken over. "Back" passed in 3.0 s.
A lock held by a live process that released it after about 43 s "Back", whose timeout is 30 s, waited and then passed in 41.1 s.
Wait limit lowered to 10 s (not committed), with the lock held by a live process The test failed after 10.1 s with Gave up after 10 s waiting for the USER role profile lock <dir>, held by pid <pid>. The holder's lock was left in place.
"Back" with a 60 s pause added after it creates the profile (not committed), run with the import test, --workers=2 "Back" timed out at 30 s. Its afterEach still deleted the profile and released the lock. The import test, which had been waiting, then took the lock, created its own profile and passed.

The runs in the This branch table above were made before the lock was added.

At the head of this PR, npx tsc --noEmit (which covers e2e/), npx eslint src/ e2e/ --max-warnings 0 and prettier on the changed files pass. bun run test:lint-rules, npx vitest run (154 files, 3338 tests) and bun run build passed before the lock was added. The lock changes only files in e2e/, which vitest excludes.

Test Configuration:

  • E2E_LIVE=1 E2E_BASE_URL=<panel> E2E_ADMIN_EMAIL=<admin> E2E_ADMIN_PASSWORD=<password> npx playwright test e2e/config-shell.spec.ts e2e/profiles.spec.ts e2e/import.spec.ts --workers=2
  • profiles.spec.ts also needs E2E_PROFILE_USER, set to the email of a user that has no profile.
  • The race runs: ... npx playwright test e2e/config-shell.spec.ts e2e/import.spec.ts e2e/specs.spec.ts:182 --workers=4 --repeat-each=3. The before-and-after runs of the three USER profile tests used e2e/specs.spec.ts with --grep "Back after selecting|imports into an existing profile|inherits read-only".
  • Playwright 1.61, with a production build of the panel against a LibreChat dev backend.

Checklist

  • My code adheres to this project's style guidelines
  • I have performed a self-review of my own code
  • My changes do not introduce new warnings
  • I have written tests demonstrating that my changes are effective or that my feature works
  • Local unit tests pass with my changes

…eak the Live Profiles and Import Specs

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.
@danny-avila

Copy link
Copy Markdown
Contributor Author

@codex review

Pass 1. Head: 79a953a

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Oct 27, 2026 •

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ✅ Completed 2026-10-27T12:19:30.741252Z 74fe97c Manual request
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 79a953ab66

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread e2e/config-shell.spec.ts Resolved
@danny-avila

Copy link
Copy Markdown
Contributor Author

@codex review

Pass 2. Head: 74fe97c

@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review: Didn't find any major issues. 🚀

Reviewed commit: 74fe97ce58

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

# Conflicts:
#	e2e/live.ts
@danny-avila
danny-avila merged commit 1027ad9 into main Oct 28, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant