From 20998040bf75230a7c4b831dd81259f7803a1e94 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:27:48 +0000 Subject: [PATCH 1/2] fix(e2e): re-query elements before acting to fix thread_resume flake `cypress/e2e/thread_resume/spec.cy.ts` failed ~20-33% of runs with a detached-DOM error when clicking `#chat-submit`, burning ~2 minutes of CI per occurrence and producing red builds that looked like real regressions on unrelated PRs. An assertion terminates a Cypress query chain: `.should()` freezes the element it resolved, so the following action command acts on that stale node instead of re-querying it. `SubmitButton` renders `#stop-button` and `#chat-submit` in different branches of a ternary, so the send button is genuinely unmounted while a task is running -- not merely disabled. On resume, `task_start`/`task_end` and `first_interaction` all fire while `@cl.on_chat_resume` runs, destroying and recreating `#chat-submit` in exactly the window between the assertion passing and the click landing. Split the chains in `submitMessage` and `openHistory` so Cypress keeps retrying the query itself. Helper signatures are unchanged, so all calling specs are fixed at once. Also wait for the thread to finish hydrating in the resume spec: `#message-composer` turns visible before resume completes, so assert the `on_chat_resume` message has landed and no task is running before submitting again. Note the test app's `MemoryDataLayer.create_step` is a no-op and threads store `steps: []`, so the first turn is not replayed on resume -- the `Resumed:` message is the signal that is actually there. Refs #3019 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Cw4RvYzeQqLrEgvX2kehYK --- cypress/e2e/thread_resume/spec.cy.ts | 16 +++++++++++++++- cypress/support/testUtils.ts | 19 +++++++++++++------ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/cypress/e2e/thread_resume/spec.cy.ts b/cypress/e2e/thread_resume/spec.cy.ts index d8e63b613a..1bbac3d888 100644 --- a/cypress/e2e/thread_resume/spec.cy.ts +++ b/cypress/e2e/thread_resume/spec.cy.ts @@ -21,9 +21,13 @@ describe('Thread resume (author)', () => { it('resumes own thread, composer visible, can continue chatting', () => { login('alice'); - // Start a thread + // Start a thread and let the first turn finish before reloading submitMessage('hi'); cy.location('pathname').should('match', /\/thread\//); + cy.get("[data-step-type='assistant_message']").should( + 'contain', + 'Echo: hi' + ); // Reload to trigger resume cy.reload(); @@ -32,6 +36,16 @@ describe('Thread resume (author)', () => { cy.get('#message-composer').should('be.visible'); cy.get('[data-testid="read-only-banner"]').should('not.exist'); + // `#message-composer` turns visible before the thread has finished + // hydrating. Wait for the `@cl.on_chat_resume` message to land and for the + // task to end (the composer shows a stop button while one is running), + // otherwise the send button re-renders under the click. + cy.get("[data-step-type='assistant_message']").should( + 'contain', + 'Resumed:' + ); + cy.get('#stop-button').should('not.exist'); + // Continue chatting submitMessage('still here'); cy.get("[data-step-type='assistant_message']").contains('Echo: still here'); diff --git a/cypress/support/testUtils.ts b/cypress/support/testUtils.ts index c0cb384a29..23e917453b 100644 --- a/cypress/support/testUtils.ts +++ b/cypress/support/testUtils.ts @@ -8,16 +8,23 @@ Cypress.on('uncaught:exception', (err) => { } }); +// An assertion terminates a query chain: `.should()` freezes the element it +// resolved, so a following action command acts on that stale node instead of +// re-querying it. When React re-renders in between (the composer swaps its send +// button for a stop button while a task is running), the node is detached and +// the action fails. Assert first, then re-query for the action so Cypress keeps +// retrying the query itself. +// https://docs.cypress.io/app/core-concepts/retry-ability export function submitMessage(message: string) { - cy.get('#chat-input') - .should('be.visible') - .should('not.be.disabled') - .type(message); - cy.get('#chat-submit').should('not.be.disabled').click(); + cy.get('#chat-input').should('be.visible').should('not.be.disabled'); + cy.get('#chat-input').type(message); + cy.get('#chat-submit').should('not.be.disabled'); + cy.get('#chat-submit').click(); } export function openHistory() { - cy.get(`#chat-input`).should('not.be.disabled').type(`{upArrow}`); + cy.get(`#chat-input`).should('not.be.disabled'); + cy.get(`#chat-input`).type(`{upArrow}`); } export function closeHistory() { From c13ae2588908a43b357683bb8eb3f0cdd9e4e95f Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 26 Aug 2026 09:45:47 +0000 Subject: [PATCH 2/2] fix(e2e): wait for the resume replay, not the message it erases The previous commit's post-reload waits were built on a misreading of the resume path; an adversarial review refuted both. `connection_successful` (socket.py:208-240) emits, in order: `first_interaction`, then the `@cl.on_chat_resume` message, then `resume_thread`. The client's `resume_thread` handler rebuilds the message list from `thread.steps` and *replaces* it wholesale. So the `Resumed: ...` message this spec waited for is erased milliseconds after it renders -- the assertion could only pass by sampling a sub-frame window. And there is no `task_start` on the resume path at all (only `task_end`, up front), so `#stop-button` never exists and asserting its absence guarded nothing. The real churn is composer ownership: `Footer` renders nothing while the message list is empty and `WelcomeScreen` renders its own `MessageComposer` instead, so every flip of `hasMessage()` remounts the input. With `create_step` a no-op and `steps` always `[]`, the replay drove the list back to empty and swapped the composer a second time -- landing squarely in the window the old assertion opened. Persist steps in the fixture's `MemoryDataLayer` so resume replays a real history. That gives the swap a stable end state (`hasMessage()` stays true across the replay) and makes the spec exercise thread resume rather than resuming an empty thread. Wait on `Echo: hi`, which only the replay can produce and which is the last write to the list. Also wait for the `on_chat_start` welcome message before the first submit: the same swap races `type()` on the first turn, dropping the typed value and leaving the send button disabled. The `testUtils` comment claimed `.should()` freezes its subject. The Cypress docs say no such thing -- they say queries are retried but action commands are not re-run, and recommend a separate `cy.get()` per action. Keep the split, correct the rationale. Refs #3019 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Cw4RvYzeQqLrEgvX2kehYK --- cypress/e2e/thread_resume/main.py | 70 +++++++++++++++++++++------- cypress/e2e/thread_resume/spec.cy.ts | 27 ++++++++--- cypress/support/testUtils.ts | 16 ++++--- 3 files changed, 83 insertions(+), 30 deletions(-) diff --git a/cypress/e2e/thread_resume/main.py b/cypress/e2e/thread_resume/main.py index 1f96fa4952..c797ddec19 100644 --- a/cypress/e2e/thread_resume/main.py +++ b/cypress/e2e/thread_resume/main.py @@ -3,6 +3,7 @@ import chainlit as cl import chainlit.data as cl_data +from chainlit.data.utils import queue_until_user_message from chainlit.element import Element, ElementDict from chainlit.step import StepDict from chainlit.types import ( @@ -23,6 +24,42 @@ THREADS: Dict[str, List[ThreadDict]] = {} +def find_thread(thread_id: str) -> Optional[ThreadDict]: + """Return the stored thread with this id, whichever user bucket holds it.""" + for threads in THREADS.values(): + for thread in threads: + if thread["id"] == thread_id: + return thread + return None + + +def ensure_thread(thread_id: str, user_id: Optional[str] = None) -> ThreadDict: + """Return the stored thread with this id, creating the row if needed. + + Steps can be persisted before the first `update_thread` call, so a row may + have to be created without an owner. Re-home it when the owner shows up + instead of appending a second, empty row under the user. + """ + thread = find_thread(thread_id) + if thread is None: + thread = { + "id": thread_id, + "createdAt": utc_now(), + "userId": user_id, + "userIdentifier": user_id, + "name": thread_id, + "steps": [], + } + THREADS.setdefault(user_id or "", []).append(thread) + elif user_id and thread.get("userIdentifier") != user_id: + for uid, threads in THREADS.items(): + THREADS[uid] = [t for t in threads if t is not thread] + thread["userId"] = user_id + thread["userIdentifier"] = user_id + THREADS.setdefault(user_id, []).append(thread) + return thread + + class MemoryDataLayer(cl_data.BaseDataLayer): async def get_user(self, identifier: str): return cl.PersistedUser(id=identifier, createdAt=now, identifier=identifier) @@ -55,14 +92,26 @@ async def get_element( async def delete_element(self, element_id: str, thread_id: Optional[str] = None): pass + @queue_until_user_message() async def create_step(self, step_dict: "StepDict"): - pass - + thread_id = step_dict.get("threadId") + if not thread_id: + return + steps = ensure_thread(thread_id)["steps"] + for index, existing in enumerate(steps): + if existing["id"] == step_dict["id"]: + steps[index] = step_dict + return + steps.append(step_dict) + + @queue_until_user_message() async def update_step(self, step_dict: "StepDict"): - pass + await self.create_step(step_dict) + @queue_until_user_message() async def delete_step(self, step_id: str): - pass + for thread in (t for threads in THREADS.values() for t in threads): + thread["steps"] = [s for s in thread["steps"] if s["id"] != step_id] async def get_thread_author(self, thread_id: str) -> str: return (await self.get_thread(thread_id))["userIdentifier"] @@ -96,18 +145,7 @@ async def update_thread( metadata: Optional[Dict] = None, tags: Optional[List[str]] = None, ): - user_threads = THREADS.setdefault(user_id or "", []) - thr = next((t for t in user_threads if t["id"] == thread_id), None) - if not thr: - thr = { - "id": thread_id, - "createdAt": utc_now(), - "userId": user_id, - "userIdentifier": user_id, - "name": name or thread_id, - "steps": [], - } - user_threads.append(thr) + thr = ensure_thread(thread_id, user_id) if name: thr["name"] = name if metadata is not None: diff --git a/cypress/e2e/thread_resume/spec.cy.ts b/cypress/e2e/thread_resume/spec.cy.ts index 1bbac3d888..b588d36369 100644 --- a/cypress/e2e/thread_resume/spec.cy.ts +++ b/cypress/e2e/thread_resume/spec.cy.ts @@ -21,7 +21,18 @@ describe('Thread resume (author)', () => { it('resumes own thread, composer visible, can continue chatting', () => { login('alice'); - // Start a thread and let the first turn finish before reloading + // `on_chat_start` sends its welcome message asynchronously. While the + // message list is empty `WelcomeScreen` renders the composer and `Footer` + // renders nothing; the first message flips `hasMessage()` and swaps which + // of the two owns it, remounting the input. Wait for that swap before + // typing, or the typed value is dropped mid-`type()`. + cy.get("[data-step-type='assistant_message']").should( + 'contain', + 'Welcome, say hi to start!' + ); + + // Start a thread and let the first turn finish, so the data layer has + // persisted it and there is a history for resume to replay. submitMessage('hi'); cy.location('pathname').should('match', /\/thread\//); cy.get("[data-step-type='assistant_message']").should( @@ -36,15 +47,17 @@ describe('Thread resume (author)', () => { cy.get('#message-composer').should('be.visible'); cy.get('[data-testid="read-only-banner"]').should('not.exist'); - // `#message-composer` turns visible before the thread has finished - // hydrating. Wait for the `@cl.on_chat_resume` message to land and for the - // task to end (the composer shows a stop button while one is running), - // otherwise the send button re-renders under the click. + // `#message-composer` is visible well before the thread has hydrated. The + // backend's resume path emits, in order: the `@cl.on_chat_resume` message, + // then `resume_thread` with the persisted history -- and the client's + // `resume_thread` handler *replaces* the whole message list with it. So + // `Echo: hi` can only come from that replay, which is the last write to + // the list; once it is on screen every resume event has landed and the + // composer has stopped remounting. cy.get("[data-step-type='assistant_message']").should( 'contain', - 'Resumed:' + 'Echo: hi' ); - cy.get('#stop-button').should('not.exist'); // Continue chatting submitMessage('still here'); diff --git a/cypress/support/testUtils.ts b/cypress/support/testUtils.ts index 23e917453b..b3fd80abfa 100644 --- a/cypress/support/testUtils.ts +++ b/cypress/support/testUtils.ts @@ -8,13 +8,15 @@ Cypress.on('uncaught:exception', (err) => { } }); -// An assertion terminates a query chain: `.should()` freezes the element it -// resolved, so a following action command acts on that stale node instead of -// re-querying it. When React re-renders in between (the composer swaps its send -// button for a stop button while a task is running), the node is detached and -// the action fails. Assert first, then re-query for the action so Cypress keeps -// retrying the query itself. -// https://docs.cypress.io/app/core-concepts/retry-ability +// The composer is re-rendered by React while a message round-trip is in flight, +// so the input and the send button can be replaced between the two actions +// below. Cypress retries queries but never re-runs an action command, so the +// docs recommend a separate `cy.get()` per action rather than one chain: +// "Call cy.get() separately for each action if the DOM node might be replaced +// between steps." -- https://docs.cypress.io/app/core-concepts/retry-ability +// ("Use separate queries for re-rendering elements"); see also +// https://docs.cypress.io/app/core-concepts/interacting-with-elements +// ("Detached"). export function submitMessage(message: string) { cy.get('#chat-input').should('be.visible').should('not.be.disabled'); cy.get('#chat-input').type(message);