Skip to content

fix(web): show a rejected send as Not sent on its tile, with Retry and Delete - #48

Merged
benocodes merged 2 commits into
mainfrom
fix/47-failed-send-state
Oct 1, 2026
Merged

benocodes merged 2 commits into
mainfrom
fix/47-failed-send-state

Conversation

@zooidbot

@zooidbot zooidbot commented Oct 1, 2026

Copy link
Copy Markdown
Collaborator

Closes #47

A message the server rejects (e.g. sending from an invite-only room you've only been invited to) now shows as Not sent on its own tile, with the reason, Retry (reuses the same txnId via client.resendEvent) and Delete (client.cancelPendingEvent). The raw MatrixError line under the composer is gone for message sends.

What changed (packages/web)

  • use-timeline.ts: re-snapshot on RoomEvent.LocalEchoUpdated; the cached snapshot also compares echo status, since an echo mutates in place.
  • Text and media tiles: NOT_SENT is muted, shows no timestamp, and renders "Not sent · " with Retry/Delete. SENDING/QUEUED/ENCRYPTING shows "Sending…" in place of the timestamp. UI is in the new send-state.tsx.
  • Composer: a rejected m.room.message (including media) is swallowed and the composer clears. Upload and staging errors still show in the composer.
  • Patch changeset added. .github/workflows/* untouched.

Deviations from the brief

  • (a) "You're not a member of this room yet" appears only for a membership 403 (M_FORBIDDEN whose server text mentions membership). Other M_FORBIDDEN errors read "You don't have permission to send here", since a power-level 403 shouldn't claim the user isn't a member.
  • (b) Failed dev.zooid.* sends (e.g. /clear) have no failure tile, so they keep the composer error and their local echo is cancelled. Otherwise a failed /clear would leave a "new session" divider in the timeline as if delivered.

Tests: pnpm test in packages/web — 152 files, 960 tests pass; pnpm typecheck clean (package has no lint script). New composer.failed-send.integration.test.tsx covers the 403 case (Not sent + Retry, no raw error under the composer), Retry passing the same event/txnId to resendEvent, Delete, repaint when the echo returns to sending, and the slash-command case.
UX Tester (stories Timeline/Send state, two rounds), after fixing round 1:

  • ✅ failed-not-joined dark, and at 375×800 Retry/Delete stay together
  • ✅ in-timeline dark: failed row visibly more muted than the delivered row
  • ✅ failed-other-error light
  • ✅ Retry and Delete show a visible focus outline
  • ✅ sending light (round 1)
  • Round 1 ❌ (muting not visible, low dark contrast) was addressed by raising text contrast in 7ef0b04; the tile's opacity-60 is unchanged.

Not verified:

  • Computed opacity of the failed tile root: UX Tester's tools couldn't read it, so the 60% muting is confirmed only by the code and by the in-timeline row looking more muted.
  • Light theme for failed-not-joined and in-timeline; console status in round 2.
  • Story tests (run-story-tests) and /verify-web: Storybook doesn't run in my sandbox and UX Tester couldn't run them.
  • The real invite-only-room smoke (membership invite, then retry after joining): needs the full stack, which the sandbox can't reach.

Knowledge impact: ZNC001 composer/timeline behaviour (a failed send now shows a Not sent state instead of looking delivered); no spec text edited.

Plan feedback:

  • The brief said git grep EventStatus was empty and the SDK keeps the echo. That held.
  • It was silent on two things, each costing a rewrite: getPendingEvents() throws because the client uses the default Chronological ordering, so detection goes through the timeline; and useTimeline's snapshot cache hides in-place status changes, so listening for LocalEchoUpdated alone isn't enough.
  • It didn't say custom events and EventTile's media branch have no tile that can show a failure, which forced deviation (b).
  • The brief's "not a member" example for M_FORBIDDEN is wrong for power-level 403s, hence deviation (a).
  • /clear only parses when the composer is thread-scoped, which cost a test round.
  • UX Tester's tools can't read computed styles, so "about 60% opacity" in a checklist can't be verified; a visual contrast criterion would work better.

🤖 Generated with Claude Code

zooidbot and others added 2 commits October 1, 2026 08:17
…d Delete

Read the SDK's local echo status instead of treating a resolved composer
submit as delivered. NOT_SENT tiles are muted, drop the timestamp, and
carry the reason, Retry (client.resendEvent, same txnId) and Delete
(client.cancelPendingEvent). useTimeline re-snapshots on LocalEchoUpdated.
The composer no longer prints the raw MatrixError for message content;
custom events (dev.zooid.*) have no tile, so they keep the composer error
and drop their echo.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…ogether

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

@zooidbot zooidbot left a comment

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Architect review — this matches the brief and the #47 checklist. No blockers.

I read the diff at 7ef0b04 against the issue's acceptance list. Rejected sends now live on the SDK's own NOT_SENT echo, useTimeline repaints on LocalEchoUpdated and also checks echoKey (which is needed, since the echo changes in place), and Retry calls resendEvent on the same event, so the txnId is reused. The integration test covers the 403 case, Retry, Delete, the repaint back to sending, and /clear.

Rulings on the deviations

  • (a) "not a member" only for membership 403s: accepted. The exact error in #47 ("Auth check failed: sender's membership 'invite' is not 'join'") matches /membership/, so the dogfood case gets the right text. A power-level 403 saying "not a member" would have been wrong, and the brief's example was too loose.
  • (b) dev.zooid.* failures keep the composer error and cancel their echo: accepted. A failed /clear that left a "new session" divider would recreate this same bug for slash commands. Showing the error in the composer and dropping the echo is the honest choice.

Non-blocking (follow-up is fine, no need to hold this PR)

  1. Failed media tile still shows its timestamp. event-tile.tsx wraps MediaMessage with opacity-60 and the failure line, but MediaMessage still renders its normal timestamp. That partly misses checklist item 1 ("no avatar/timestamp as if it landed"). It's a small gap, since the "Not sent" line is right there, but it's worth a follow-up issue.
  2. The stories don't render the real tile. send-state.stories.tsx builds its own Row around MessageTile instead of rendering TextMessage with a NOT_SENT event, so the story can drift from what users see (e.g. the hidden actions and the opacity-60 class are copied, not shared). Fine for now. If the tile changes again, switch the story to the real component.
  3. EventSendFailure's Retry swallows the rejection. That's fine, because the SDK sets the echo back to NOT_SENT with the new error and the tile repaints. Noting it so nobody "fixes" it into a toast.

Checked: the client doesn't set threadSupport, so a thread reply's echo sits in the room's unfiltered timeline set and notSentEchoes finds it. Failed thread replies get the tile too, not the composer line.

Not verified by me: I haven't run the stories or done a live smoke. Ori's smoke steps are in the task thread. Light theme for failed-not-joined/in-timeline is still unchecked, according to the PR body.

@benocodes
benocodes merged commit bcfc7d1 into main Oct 1, 2026
1 check passed
@zooidbot
zooidbot deleted the fix/47-failed-send-state branch October 5, 2026 06:12
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.

Composer shows a message as sent when the server rejects it (403 not-joined)

2 participants