fix(e2e): de-flake thread_resume by waiting on the resume replay

[dokterbob]

Fixes #3019.

What the flake actually is

The issue diagnosed a stale-subject bug in submitMessage — cy.get('#chat-submit').should('not.be.disabled').click() acting on an already-resolved node. That's a real smell and is fixed here, but it isn't the mechanism.

Composer ownership moves between two different components as the message list changes:

• frontend/src/components/chat/Footer.tsx:20 returns null while !hasMessage(messages)
• frontend/src/components/chat/WelcomeScreen.tsx:80,91 returns null once hasMessage(messages), and otherwise renders its own MessageComposer

So every flip of hasMessage() unmounts one composer and mounts another. Two points in this spec crossed that boundary mid-interaction:

1. First turn. on_chat_start sends its welcome message asynchronously. Landing during type() remounts the input, loses the typed value, and leaves #chat-submit disabled.
2. Resume. connection_successful (backend/chainlit/socket.py:208-240) emits first_interaction → the @cl.on_chat_resume message → resume_thread. The client handler (libs/react-client/src/useChatSession.ts:258+) rebuilds the list from thread.steps and replaces it wholesale. Because this fixture's MemoryDataLayer.create_step was a no-op and steps was always [], that replay drove the list back to empty — swapping the composer a second time, exactly where the test clicks.

cy.get('#message-composer').should('be.visible') is satisfied early in that sequence, well before hydration finishes, so it never gated anything.

The underlying app behaviour is filed separately as #3022 — this PR makes the test honest about the churn, it does not remove the churn.

Changes

cypress/e2e/thread_resume/main.py — implement create_step / update_step / delete_step on the fixture's MemoryDataLayer so steps are actually persisted, with @queue_until_user_message() matching data/sql_alchemy.py:383-421. ensure_thread backfills a row when a step is persisted before the first update_thread, and re-homes it when the owner arrives rather than duplicating it.

This is the crux, not incidental cleanup. With steps: [] the resume ends in a state indistinguishable from a fresh chat, so there was no observable signal that hydration had finished to wait on. Persisting steps gives the replay a stable end state — hasMessage() now stays true across it, so the composer no longer remounts — and means the spec finally exercises thread resume instead of resuming an empty thread.

cypress/e2e/thread_resume/spec.cy.ts — wait for the welcome message before the first submit, and after reload wait on Echo: hi, which only the replay can produce and which is the last write to the message list. No cy.wait(<ms>).

cypress/support/testUtils.ts — split the query chains in submitMessage / openHistory so each action re-queries. Signatures unchanged; all 34 call sites unaffected.

Verification

Local: pnpm lint cypress/, Prettier, and ruff check / ruff format --check all clean. Note pnpm type-check does not cover cypress/.

CI: three attempts of run 32954779496, all on this commit, all green. Because retries: 3 would mask a surviving flake behind a green badge, I pulled all 30 shard logs and checked (Attempt N of 4) annotations directly:

Attempt	thread_resume (ubuntu-5)	thread_resume (windows-5)	Retries in run
1	✓ first attempt, 3664ms	✓ first attempt, 4143ms	none
2	✓ first attempt, 4150ms	✓ first attempt, 3399ms	none
3	✓ first attempt, 3032ms	✓ first attempt, 4594ms	one, unrelated (see below)

Six first-attempt passes, no retry annotations on this spec anywhere. All 30 shard-runs ended Failing: 0.

Being straight about the strength of this: at the 20–33% rate reported in #3019, three clean runs would happen by luck roughly 30–50% of the time, so the runs alone aren't proof. The mechanism argument is the stronger half — hasMessage() no longer flips during the resume, so the remount that detached the button cannot occur — and the retry data rules out the specific way a survivor would have hidden.

Attempt 3 did contain one genuine retry, in oauth_auth on windows-latest-3, unrelated to this spec and pre-existing. Filed as #3023, with the general "retries are invisible in CI" problem as #3024.

Review notes

• 2099804 is wrong and c13ae25 corrects it; left in history rather than force-pushed so the reasoning is visible. Happy to squash on merge.
• Known-remaining, deliberately out of scope: submitMessage is still exposed to the same remount race in other specs that call it against an empty message list (Composer remounts whenever the message list transitions empty↔non-empty, dropping in-progress input #3022 is the real fix); [data-testid="read-only-banner"] at spec.cy.ts:48 asserts a testid that exists nowhere in the frontend and is therefore vacuous (noted in Surface Cypress retries in CI so flaky specs stop hiding behind green runs #3024); ~27 other .should(...).<action>() sites remain across cypress/e2e/.

[dokterbob]

@codex review

[Copilot]

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

[cubic-dev-ai]

1 issue found across 3 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="cypress/e2e/thread_resume/main.py">

<violation number="1" location="cypress/e2e/thread_resume/main.py:54">
P3: ensure_thread re-homes any thread whose stored userIdentifier differs from the user_id passed to update_thread, moving it out of whichever bucket currently holds it. In this shared multi-user store (alice and bob authenticate against the same global THREADS), an update_thread issued with a different owner_id would silently reassign another user's thread to the caller. The owner is the session user and is fixed per thread, so this is latent for the current test, but the guard offers no protection if a thread id is ever referenced from a second user's session. Consider skipping the re-home when the thread already has an owner, or guarding on ownership rather than unconditional reassignment.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P3: ensure_thread re-homes any thread whose stored userIdentifier differs from the user_id passed to update_thread, moving it out of whichever bucket currently holds it. In this shared multi-user store (alice and bob authenticate against the same global THREADS), an update_thread issued with a different owner_id would silently reassign another user's thread to the caller. The owner is the session user and is fixed per thread, so this is latent for the current test, but the guard offers no protection if a thread id is ever referenced from a second user's session. Consider skipping the re-home when the thread already has an owner, or guarding on ownership rather than unconditional reassignment.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At cypress/e2e/thread_resume/main.py, line 54:

<comment>ensure_thread re-homes any thread whose stored userIdentifier differs from the user_id passed to update_thread, moving it out of whichever bucket currently holds it. In this shared multi-user store (alice and bob authenticate against the same global THREADS), an update_thread issued with a different owner_id would silently reassign another user's thread to the caller. The owner is the session user and is fixed per thread, so this is latent for the current test, but the guard offers no protection if a thread id is ever referenced from a second user's session. Consider skipping the re-home when the thread already has an owner, or guarding on ownership rather than unconditional reassignment.</comment>

<file context>
@@ -23,6 +24,42 @@
+            "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]
</file context>