Skip to content

fix(messages): tolerate a cancelled reaction-name chunk load - #3450

Merged
matt2e merged 2 commits into
mainfrom
mtoohey/flake-reaction-name-chunk
Nov 14, 2026
Merged

fix(messages): tolerate a cancelled reaction-name chunk load#3450
matt2e merged 2 commits into
mainfrom
mtoohey/flake-reaction-name-chunk

Conversation

@matt2e

@matt2e matt2e commented Nov 13, 2026

Copy link
Copy Markdown
Contributor

Why

user-status.spec.mjs, "statuses edit, synchronize, clear, reject stale traffic and retain failed drafts", fails intermittently on WebKit with an unexplained page error: Unhandled Promise Rejection: TypeError: Importing a module script failed.

Sighting Lane
9 Nov, PR, and again on its re-run Flake check, WebKit 2/11
10 Nov, main Browser journeys, WebKit 9/18
11 Nov, PR Flake check, WebKit 2/12

The spec's check for unexplained page errors is deliberate and correct. The bug is in the product.

Root cause

useReactionName and ReactionEmoji's revealName both ran void import("./reaction-name").then(...) with no rejection handler. reaction-name.ts statically imports the emoji-mart native table, so the first Unicode status or reaction name starts a large chunk load. When a navigation lands while that load is in flight, WebKit cancels the request, the import() rejects, and the page reports an unhandled rejection.

In the journey, the click that opens a free-text status starts the load and the next page.goto cancels it. The traces show the chunk request failing with "Load request cancelled" and the page error 4 ms after the navigation. The unguarded import arrived with #2384 and moved into useReactionName in #2774.

Fix

Handle the rejection at both sites.

  • A failed load leaves the name unresolved. In the hook the next mount tries again. ReactionEmoji keeps the raw content and the next hover or focus tries again.
  • Both handlers console.warn with the error, so a chunk that keeps failing, such as a 404 after a deploy with a stale shell, stays visible. The comments note that only a new document fetches the chunk again, because some engines keep a failed module fetch for the document's life.

The warning does not affect browser checks: the fixture records only error-level console messages, and the page-error helper watches pageerror. There is no spec or fixture change, and the page-error check stays as it is.

The second commit is the review follow-up that replaced the first commit's empty handlers with the warning and made the two unit tests wait for it.

Tests

  • use-reaction-name.test.tsx (new) and a MessageReactions.test.tsx case mock ./reaction-name to reject. They assert the name stays unresolved, that the warning is logged, and that no unhandled rejection reaches the process. They then unmock and show the next mount or reveal resolves.
  • On the unfixed code both fail with the unhandled rejection.

Evidence

Scope Result
This branch's head, macOS: bin/pnpm exec vitest run src/features/messages/use-reaction-name.test.tsx src/features/messages/MessageReactions.test.tsx 2 files, 29 passed
Before the split from a combined branch, Linux: the same two files 27 passed in 8 of 8 runs, no stderr output
Linux, WebKit: user-status.spec.mjs, --workers=1 --repeat-each=8 8/8 passed, 11.7 to 12.7 s each
Triage, WebKit, with the native table request held across the navigation unfixed 3/3 failed with the CI error, fixed 3/3 passed
Before the split, Linux: Biome on the touched files, tsc --noEmit clean

The count is 29 here and 27 in the commit messages because #3397 has since added tests to MessageReactions.test.tsx.

Not reproduced, not run

  • The cancelled chunk did not reproduce on an idle host without holding the request, so the WebKit repeat guards against regression rather than reproducing CI timing.
  • Not tried in the desktop app. The user-visible change is the absence of an unhandled rejection, plus a console warning when the chunk fails.
  • The local pre-push checks for this branch (whole-project type check, related unit tests, design-system check) were not run. The two-file run in the first row is the only check run on this branch's head. The type check and Biome results are from the combined branch, where the same change passed.
  • No human has tested this yet, so buzz-review-completed is not set.

Left alone

🤖 Generated with Claude Code

matt2e and others added 2 commits November 13, 2026 13:42
@matt2e @claude
fix(messages): tolerate a cancelled reaction-name chunk load
82fde4a 
useReactionName and ReactionEmoji's revealName both ran
`void import("./reaction-name").then(...)` with no rejection handler.
reaction-name.ts statically imports the emoji-mart native table, so the
first Unicode status or reaction name starts a large chunk load. When a
navigation lands while that load is in flight, WebKit cancels the request,
the import() rejects, and the page reports "Unhandled Promise Rejection:
TypeError: Importing a module script failed." In the user-status journey
the "Free-text Bob" click starts the load and the next page.goto cancels
it (the traces show "Load request cancelled" 4 ms after the goto), and the
spec's deliberate check for unexplained page errors failed the test on
WebKit in CI.

Handle the rejection at both sites. A failed load leaves `loaded`
undefined, so the next mount of the hook tries again; ReactionEmoji keeps
the raw content and the next hover or focus tries again. No spec or
fixture change: the page-error check stays as it is.

Unit coverage: use-reaction-name.test.tsx and a MessageReactions case
mock ./reaction-name to reject, assert the name stays unresolved with no
unhandledRejection reaching the process, then unmock and show the next
mount/reveal resolves (:house: / :+1:). On the unfixed code both fail
with the unhandled rejection; with the fix all 27 tests in the two files
pass.

Verified with `bin/pnpm test:browser:ci --project webkit --no-deps
--workers=1 --repeat-each=8 --reporter=list
tests/browser/user-status.spec.mjs`: 8/8 passed locally (11.7–12.7 s
each, 1.7 m). The cancelled chunk did not reproduce on an idle host; the
triage reproduced it deterministically by holding the native.json request
across the navigation (3/3 fail unfixed, 3/3 pass fixed), so the local
repeat guards against regression rather than reproducing CI timing. Biome
and tsc --noEmit are clean.

The separate "Duration: Today" vs "8 hours" tie in statusDuration at
exactly 16:00 UTC is not addressed here.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
@matt2e @claude
fix(messages): warn when the reaction-name chunk fails to load
cf0bd7f 
The rejection handlers added for a cancelled reaction-name chunk load
were empty, which made a chunk that keeps failing, such as a 404 after a
deploy with a stale shell, silent. useReactionName and ReactionEmoji's
revealName now console.warn with the error, as the rest of src does. No
browser check changes: the fixture records only error-level console
messages and page-errors.mjs watches pageerror.

The comments now say that only a new document fetches the chunk again,
since some engines keep a failed module fetch for the document's life
and reject at once.

The two unit tests spy on console.warn and assert the warning, waiting
for it with waitFor: the mocked import rejects after a nondeterministic
number of ticks and a single flush was not always enough (one of six
local runs saw no call at assertion time before the wait was added).

Verified as part of the combined review follow-up, before it was split
by path:
- vitest run use-reaction-name.test.tsx MessageReactions.test.tsx: 27
  passed in 8 of 8 runs, with no stderr output.
- bin/pnpm test:browser:ci --project webkit --no-deps --workers=1
  tests/browser/user-status.spec.mjs
  tests/browser/composer-attachments.spec.mjs: 11 passed (1.4 m),
  including user-status's unexplained page-error check.
- biome check --error-on-warnings on the touched files and tsc --noEmit
  are clean.

Co-Authored-By: Claude Fable 5.1 <[email protected]>
Signed-off-by: mtoohey <[email protected]>
Signed-off-by: Matt Toohey <[email protected]>
@matt2e
matt2e requested a review from a team as a code owner November 13, 2026 03:12
kazemek21
kazemek21 approved these changes Nov 13, 2026
View reviewed changes

@kazemek21 kazemek21 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Kyle reviewed the PR and the Nexus review-tracker notes. Approved with no comments.

Posted by Codex (gpt-6.1-sol) on behalf of kylez.

wesbillman
wesbillman reviewed Nov 13, 2026
View reviewed changes

@wesbillman wesbillman left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

🤖 Star Lord automated source review via Wes’s account.

No actionable findings at cf0bd7fe53810a1835cc77600a3e7d5bb6f14583 against base/merge-base 7f03764beb8eb0ddcb4ff9ec0b8bda3e07699716.

Both dynamic imports handle rejection while preserving the existing lazy-load triggers, successful lookup behavior and raw/unresolved fallback. The hook retains its unmount guard. The new tests await the failure handler before checking fallback, then exercise another mount or focus reveal.

Source-only review: no tests, builds or app execution performed. The mocked retry cases do not establish browser refetch after a cached module failure, or reproduce WebKit navigation cancellation. Hosted checks in the inspected current-head snapshot passed, with Windows native validation skipped; desktop, screen-reader and human acceptance remain unverified. The PR description distinguishes current-head checks from earlier combined-branch evidence.

@matt2e
matt2e merged commit 801a3d4 into main Nov 14, 2026
62 checks passed
@matt2e
matt2e deleted the mtoohey/flake-reaction-name-chunk branch November 14, 2026 00:46
wpfleger96 added a commit that referenced this pull request Nov 14, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into duncan/cross-communit…
3cec1f1 
…y-unreads

* origin/main: (24 commits)
  [1/2] feat(agents): add a recommended agent setup (core) (#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (#3444)
  Expose native channel headings to Solo sources (#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (#3447)
  fix(solo): keep every contributed destination action (#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (#2453)
  fix(profiles): open avatar previews on hover in compact density (#3510)
  feat(solo): smooth chat recency reordering (#3502)
  Reject self-mentions that cannot wake another session (#3513)
  feat(status): offer don't clear for every status (#3515)
  Let plugin panels name their tab with panels.useTabTitle (#3251)
  fix(agents): report a conversation wake that fails before its turn starts (#2192)
  ...

# Conflicts:
#	tests/browser/message-management.spec.mjs
wpfleger96 added a commit that referenced this pull request Nov 14, 2026
@wpfleger96
Merge remote-tracking branch 'origin/main' into duncan/clock-skew-2min
7c356a2 
* origin/main: (27 commits)
  [2/2] feat(agents): offer Automatic (Recommended) in Create agent and Identity (#3345)
  fix(solo): preserve and resume unsent chat drafts (#3525)
  fix(builderlab): give refused app sign-ins and Identity links a route to Organization sign-in (#3078)
  fix(agents): say why Import an agent can't import here, and link Manage plugins (#3073)
  [1/2] feat(agents): add a recommended agent setup (core) (#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (#3444)
  Expose native channel headings to Solo sources (#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (#3447)
  fix(solo): keep every contributed destination action (#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (#2453)
  fix(profiles): open avatar previews on hover in compact density (#3510)
  feat(solo): smooth chat recency reordering (#3502)
  ...

# Conflicts:
#	src-tauri/src/federated_identity/agent/tests.rs
#	src-tauri/src/federated_identity/mod.rs
#	src-tauri/src/federated_identity/protocol.rs
joahg added a commit to joahg/buzz-app that referenced this pull request Nov 14, 2026
@joahg
Merge remote-tracking branch 'origin/main' into joah/agents-solo-not-dm
0a821dc 
* origin/main: (133 commits)
  Expose ordinary chats without workspace headings (block#3530)
  [2/2] feat(agents): offer Automatic (Recommended) in Create agent and Identity (block#3345)
  fix(solo): preserve and resume unsent chat drafts (block#3525)
  fix(builderlab): give refused app sign-ins and Identity links a route to Organization sign-in (block#3078)
  fix(agents): say why Import an agent can't import here, and link Manage plugins (block#3073)
  [1/2] feat(agents): add a recommended agent setup (core) (block#3344)
  test(browser): survive the live-stream restart a page-clock jump causes (block#3442)
  chore(renovate): hold rusqlite below 0.41 for mesh-llm (block#3438)
  test(browser): serve the diffs and CLI reload fixtures outside the navigation budget (block#3445)
  test(browser): play the draft clip at rate 0 instead of looping it before the Play click (block#3446)
  fix(status): reopen a status whose preset deadlines coincide as Today (block#3449)
  fix(messages): tolerate a cancelled reaction-name chunk load (block#3450)
  fix(pairing): tolerate /proc/net/tcp rows printed twice or missed (block#3451)
  test(browser): wait on the signal each test asserts in local-agent-activity and message-navigation (block#3444)
  Expose native channel headings to Solo sources (block#3523)
  test(browser-host): run the 100,000-channel broker scale check alone in the measurements lane (block#3447)
  fix(solo): keep every contributed destination action (block#3516)
  fix(identity): report an issuer that keeps rate-limiting an agent as a limit (block#2365)
  fix(identity): let a stuck at-launch sign-in copy its link (block#2453)
  fix(profiles): open avatar previews on hover in compact density (block#3510)
  ...

Signed-off-by: Joah Gerstenberg <[email protected]>

# Conflicts:
#	src/bundled/codex/prompt.test.ts
#	src/bundled/goose/runtime.ts
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@wesbillman wesbillman wesbillman left review comments
@kazemek21 kazemek21 kazemek21 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@matt2e @wesbillman @kazemek21