feat(chat-frontend): room operations overhaul — async-job protocol, member roster, encryption resilience#178
Merged
Conversation
Introduces lib/asyncJob.js with requestWithAsyncResult(nc, account,
subject, payload, opts) — bridges the room-service two-phase reply
contract: a sync NATS reply (typically {status:"accepted"}) plus an
async AsyncJobResult that arrives later on
chat.user.{account}.response.{requestID}. Subscribes to the response
subject before publishing to avoid the race; injects the X-Request-ID
header that room-worker keys the AsyncJobResult publish on.
Thrown errors carry .kind from ASYNC_JOB_ERROR_KINDS (SyncError /
AsyncError / AsyncTimeout / SubscriptionClosed) so callers can branch
on category without string-matching the message. formatAsyncJobError
produces user-facing text that distinguishes wire-level failures from
server-supplied messages, and an exhaustiveness test guards against
silent drift when a new kind is added.
Also lands the supporting library pieces:
* lib/constants.js — ERR_DM_ALREADY_EXISTS + isDMExistsReply(reply),
the dedup-as-success predicate co-located with the constant it
tests. Mirrors room-service/helper.go.
* lib/subjects.js — roomCreate, memberList, userResponse builders.
* lib/useDebouncedSearch.js — null-fetcher short-circuit + JSDoc'd
contract; functional-update guards on the no-op render paths.
https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
Adds a context method that injects ncRef.current + user.account into the asyncJob helper so components can `await requestWithAsyncResult( subject, payload, opts)` without threading the NATS connection through props. Wraps the provider value in useMemo so consumers that only read stable callbacks don't re-render on every provider render — the value identity flips only when connected/user/error change. JSDoc on every callback (connectToNats, request, requestWithAsyncResult, publish, subscribe, disconnect). subscribe in particular has a non-obvious return value — the caller must .unsubscribe() to avoid leaking the iterator and the server-side sid. https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
Reusable chip-input picker shared by CreateRoomDialog and AddMembersForm. A single config-driven EntityField renders all three fields; channels get search.rooms typeahead with ChannelRef results, users and orgs are free-text-only until the server lands matching search endpoints. Spec functions (parseFreeText, entryKey, entryLabel, renderResult, entryFromResult, resultKey) are hoisted to module scope where they don't capture component state. The channel parseFreeText stays inline because it captures user.siteId. Tests cover: chip add via Enter, dedup on commit, chip removal, typeahead via search.rooms producing ChannelRef objects, fetcher error tolerance, and disabled-prop propagation. https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
…e-members tabs
Replaces the four typed-text tabs (Add / Remove / Role / Remove Org)
with two: Members (a roster) and Add.
* components/manageMembers/MemberRoster.jsx — new. Renders
member.list?enrich=true with inline Promote/Demote/Remove for
individuals and Remove for orgs. Per-section Remove-by-ID escape
hatch for accounts/orgs not surfaced in the enriched list. The
internal runAction returns Promise<boolean> so callers (e.g. the
Remove-by-ID input clearer) can branch on success rather than
chaining .then(clearInput) — that pattern would fire on failures
too because runAction catches its own rejections.
* components/manageMembers/AddMembersForm.jsx — rewrite to use
MemberPicker + requestWithAsyncResult; sends correctly-shaped
ChannelRef[] (was previously sending strings — server expects
[{roomId, siteId}]).
* components/ManageMembersDialog.jsx — tabs collapsed.
Deleted: RemoveMemberForm, RoleUpdateForm, RemoveOrgForm + their
test files. Fully subsumed by the roster.
https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
…eate subject
The dialog was publishing to chat.user.{a}.request.rooms.create with
a typed-dropdown + comma-separated-members payload, which doesn't
match any server-side handler. Server expects the room type to be
inferred from a {name, users, orgs, channels} payload shape on
chat.user.{a}.request.room.{siteId}.create, and the real outcome
arrives later as an AsyncJobResult on chat.user.{a}.response.{rid}.
This rewires CreateRoomDialog to:
* use MemberPicker — no type dropdown; type inferred server-side
(name present → channel; empty name + one user → DM or botDM,
server decides).
* use requestWithAsyncResult with isDMExistsReply as the
treatAsSuccess predicate — a sync reply of {error:"dm already
exists", roomId} lands as success and the dialog opens the
existing room.
* fall back to the counterpart account (users[0]) for the
optimistic display name until subscription.update arrives with
the canonical counterpart-derived name.
Drops the now-orphan roomsCreate subject builder and the parseList
helper (only used by the previous comma-separated-input flow, which
is gone with this commit).
https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
Two standalone smoke harnesses for the asyncJob helper, not wired
into npm test — each requires an external process and is invoked
on demand.
* scripts/asyncJob.smoke.mjs — drives requestWithAsyncResult
against a real nats-server. Verifies the X-Request-ID header
round-trip, sync + async reply shapes, the DM-exists
treatAsSuccess branch, error kinds, async-timeout cleanup, and
the subscribe-before-publish race window.
* scripts/liveStack.smoke.mjs — drives the full helper against
auth-service + room-service + room-worker on a local stack.
End-to-end create + member.list?enrich + DM dedup against real
Mongo / NATS / Valkey backing services.
https://claude.ai/code/session_01PzAJWfweSvgeX9k7j3MfDf
…dge in MessageArea header + DM name fallback
Bundles four user-visible UX gaps in the chat-frontend, all of which
were leaving users unable to complete the create-room and
manage-members flows or unable to identify DM rooms in the sidebar.
1. Free-text comma-separated entry in MemberPicker
* Each picker field (Users, Orgs, Channels) parses its input as
comma-separated values. Typing "alice, bob, charlie" + Enter
commits three chips. Whitespace around commas is trimmed and
empty segments are dropped. Channels map each typed id to a
ChannelRef with the user's siteId; cross-site picks still come
from the search dropdown.
* MemberPicker is a forwardRef component exposing
flushAndGetEntries() via useImperativeHandle. CreateRoomDialog
and AddMembersForm call it at the top of submit so typed-but-
not-Entered text becomes chips just before the request goes
out — users no longer need to remember to press Enter.
* Drops the chip-count gating on Create / Add buttons; the old
`name || chips > 0` gate left submit greyed out when the user
had typed text but not chipped it.
2. "N members" badge in the MessageArea header (replaces global header buttons)
* RoomMembersBadge fetches member.list on mount and renders
"N member(s)" as a clickable pill. Refetches on room.id change
and on a parent-controlled refreshKey bump. Hides itself for
non-channel rooms (DM membership is fixed at create).
* The badge sits inside MessageArea's header (replacing the old
static "{userCount} members" span) so it lives next to the
room it acts on. The Members and Leave buttons that used to
sit in the global chat-header are gone — the global header
carries only chat-wide actions (search, theme, logout).
LeaveRoomButton.jsx is deleted entirely; Leave is now reachable
only via the dialog's roster (self-row → Leave).
* ChatPage bumps a membersRefreshKey when ManageMembersDialog
closes so a member-add/remove inside the dialog is reflected
in the badge count immediately on dismiss.
3. Roster: Leave-for-self, owner-only Promote/Demote/Remove
* Current user's own row shows a Leave button (with window.confirm)
instead of Remove/Promote/Demote — kicking or demoting yourself
mid-flow is a UX trap.
* Promote/Demote/Remove on other rows and the Remove-by-ID inputs
are gated on isCurrentUserOwner, derived from the just-fetched
enriched roster. Non-owners see read-only info plus their own
Leave. Server still enforces auth; this is just UI parity.
* Leave goes through requestWithAsyncResult on member.remove with
the user's own account and intentionally does NOT refetch — the
dialog dismisses via ChatPage's existing
selectedRoom/summaries effect once subscription.update lands.
4. DM display name fallback
* Room.Name is empty server-side for DMs (the friendly text lives
on the user's Subscription, not the Room). RoomList and
MessageArea route the display through a new
roomDisplayName(room) helper: prefer room.name, then
room.subscriptionName, then a "(DM)" placeholder for dm/botDM,
then room.id as last-resort.
* RoomEventsContext merges evt.subscription.name onto the room
before dispatching ROOM_ADDED, so subscription.update events
populate the DM friendly name. The initial rooms.list returns
raw Rooms today (no subscription join), so pre-existing DMs
render the "(DM)" placeholder until a subscription event
refreshes them.
…age]" placeholder + diagnostic logging
broadcast-worker emits channel events in one of two shapes depending
on its BROADCAST_ENCRYPTION_ENABLED env:
* plaintext: evt.message populated with ClientMessage
* encrypted: evt.message dropped via Go's json:omitempty;
evt.encryptedMessage holds the AEAD ciphertext
The reducer's MESSAGE_RECEIVED previously silently dropped the
encrypted case (no client-side crypto). That left the room visually
frozen — the user sent a message, the server persisted + broadcast it,
the frontend received the event, the reducer dropped it, the user saw
nothing on their own screen and had to refresh to load the message via
the Cassandra-backed history RPC.
Reducer now synthesizes a placeholder message
{id: evt.lastMsgId, content: "[encrypted message]", encrypted: true}
when only encryptedMessage is present, using the top-level
lastMsgId/lastMsgAt that broadcast-worker's buildRoomEvent always
populates (regardless of encryption mode). The plaintext path is
unchanged and still wins if both fields are present
(forward-compatible with mixed rollouts, and with a future broadcaster
that ships both lanes during a rollout window).
Adds console.debug instrumentation in RoomEventsContext at every step
of the receive path:
* dmSub callback (chat.user.{account}.event.room)
* channel-sub callback (chat.room.{roomId}.event)
* openChannelSub itself, when it registers a new subscription
* reducer drops, with the reason
Each log includes subject, evt.type, roomId, lastMsgId, hasMessage,
hasEncrypted — enough to trace where an inbound event stops without
opening the WebSocket frame inspector. console.debug is hidden by
default in browser dev-tools (visible only at "Verbose"), so the
instrumentation is free in normal operation.
… ChineseName for individuals, drop Remove-by-ID forms, fix row spacing
Four roster-side polish fixes after a manual review of the
ManageMembersDialog:
1. Single ordered list (orgs first, individuals second) instead of two
separate <section>s. Same .roster-list ul, two row-renderer helpers
(renderOrgRow / renderIndividualRow). The "Members" / "Orgs"
subheaders are gone.
2. Drop the Remove-by-ID inputs (both for accounts and orgs). Every
row already has its own Remove button; the typed-id escape hatch
was redundant. Deletes the RemoveByIdRow component, its state
(removeAccountInput / removeOrgInput), and the related runAction
call sites + tests.
3. Fix row spacing for both owner and non-owner viewers. Each row is
now a flex strip with two areas:
.roster-row-info (left) — name + secondary + optional badge
.roster-row-actions (right) — buttons (or empty for non-owners)
Both areas are siblings under .roster-row { justify-content:
space-between }. The info area uses gap: 0.75rem so name /
ChineseName / [owner] never sit flush against each other,
independent of whether the actions area has buttons or is empty.
4. Individuals now show EngName (primary) + ChineseName (secondary)
instead of EngName + accountName. AccountName isn't useful in the
roster — the user already knows who they're managing by
first-name; the chineseName is a more humane secondary identifier.
Falls back gracefully: primary uses entry.engName || entry.account;
secondary span is omitted entirely when chineseName is missing.
Org rows: orgName + memberCount + Remove button.
Individual rows: engName + chineseName + Promote/Demote + Remove
(or Leave on the user's own row).
…fore closing
Previously the dialog closed as soon as the synchronous create ack
arrived. ChatPage.handleSelectRoom would set selectedRoom to the new
room, but the room wasn't in summaries yet — the server's
subscription.update event lands ~200ms later — so the existing
"auto-deselect if selectedRoom isn't in summaries" effect immediately
bounced the user back to the empty state. Even worse, the channel
subscription is opened only in the subscription.update handler, so any
message the user sent in that window was dropped on the floor with no
local echo.
Park the dialog open after sync ack with a "Waiting for server
confirmation…" button label until summaries contains the newly-minted
roomId. Two useEffects drive the close path:
* One watches `summaries` from useRoomSummaries; when it contains
pendingRoom.id (i.e. ROOM_ADDED has dispatched and openChannelSub
has fired for channels), fire onCreated + onClose.
* The other arms a 3-second safety setTimeout so a dropped
subscription.update event can't hang the dialog forever; on
timeout it closes anyway with what we already know.
By the time the dialog hands off to ChatPage:
- the room IS in summaries → ChatPage's auto-deselect effect no-ops
- the channel sub IS open → the user's first message echoes back live
Adds a useRoomSummaries mock to CreateRoomDialog.test.jsx (the dialog
now imports the hook). Default test summaries include the roomIds the
fixtures return so the happy-path tests still resolve quickly. Two new
tests cover the wait specifically: one verifies the dialog parks with
the "Waiting…" label when summaries is empty (no callbacks fire), one
advances fake timers past the 3-second timeout to verify the safety
fallback closes the dialog with the pending room payload.
… 1-10) Retrospective design doc covering all 10 commits on this branch. Captures the four user-visible problems (obsolete create-room subject, no async-result observation, wrong member payload shape, four typed-text manage-members tabs) plus the four follow-on issues that surfaced during testing (silent drop of encrypted broadcasts, create-dialog closed before subscription.update landed, DM rooms with empty server-side names, per-room controls living in the global chat-header), and how each of the ten commits addresses them. Follows the docs/superpowers/specs/ convention — Problem → Goals → Non-Goals → Design → UI placement → Trade-offs → Out of scope → File inventory. ASCII layout diagrams for the chat page, create dialog, and manage-members dialog (owner vs non-owner views) so the intent is unambiguous about where each new component lives.
…ist inline
Clicking an org row now toggles a nested list of the org's members
(EngName + ChineseName) without any per-member buttons — the parent
org row already owns the Remove control, and server treats the org as
a single membership unit. The expansion data comes from the existing
chat.user.{a}.request.orgs.{orgId}.members RPC (one-phase sync reply
returning {members: [{id, account, engName, chineseName, siteId}]}).
* lib/subjects.js: new orgMembers(account, orgId) builder mirroring
pkg/subject.OrgMembers.
* MemberRoster.jsx: three new state maps keyed by orgId —
expandedOrgs (UI open/closed), orgChildren (cached response, so
collapse + re-expand doesn't refetch), orgFetchState
('loading' | 'error'). New toggleOrg callback fetches on first
open, no-ops on subsequent re-expands. The org row is restructured
into a column with a header (info button + actions) and an
optional <ul> of children underneath; the header's info area is a
real <button aria-expanded> so keyboard nav + screen readers work.
Child rows render engName (primary) and chineseName (secondary, if
present), with no action buttons.
* styles/index.css: .roster-row-org column layout, .roster-org-toggle
button-as-label reset, .roster-chevron prefix, indented
.roster-org-children with a subtle left border so the visual
nesting is clear.
* Tests: 5 new cases — expansion fires the right subject, child rows
show EngName + ChineseName and no buttons, collapse + re-expand
reuses the cached response (only one fetch), error fallback
surfaces "Failed to load members", non-owner viewer can still
expand even though no Remove button is shown.
234 tests pass; vite build clean.
…ards + correct MessageInput placeholder
Three latent bugs in the chat-frontend, fixed together with the spec
updated to describe the actual shipped behavior.
1. CreateRoomDialog timeout fallback no longer bounces to empty state
The previous safety setTimeout called onCreated(pendingRoom) +
onClose() on timeout. That set selectedRoom on ChatPage to a room
that wasn't in summaries, so ChatPage's auto-deselect effect
immediately bounced the user back to the empty state. Even if the
bounce hadn't fired, the channel subscription wasn't opened —
subscription.update is the lane that calls openChannelSub — so any
message the user sent would not echo back live.
Timeout now surfaces an in-dialog error
("Room creation is taking longer than expected. If it succeeds,
the room will appear in your sidebar shortly — you can dismiss
this dialog.") and clears pendingRoom so the wait state unwinds.
No onCreated, no onClose. If the room was actually created, it
appears in the sidebar when subscription.update finally lands.
Decoupled `loading` (sync request in flight) from `pendingRoom`
(waiting for subscription.update) so the Cancel button is now
re-enabled during the wait; users can back out without waiting
for the 3-second timeout. Submit stays disabled the whole time so
a second click can't fire a duplicate create.
2. MemberRoster: generation guards for fetchMembers + toggleOrg
Switching rooms while member.list?enrich was in flight let the
old room's response overwrite the new room's members. Same race
on the post-action refetch and on rapidly expanding/collapsing
the same org (the per-org fetch could resolve out of order with
the user's intent).
Added memberListGenRef (one ref) and orgFetchGenRef.current keyed
by orgId. Each async fetch captures its gen at invocation and
drops the result on the floor if a later invocation has already
bumped the gen — only the latest fetch's response writes to
state.
3. MessageInput placeholder: roomPrefix + roomDisplayName
The placeholder was hardcoded `Message #${room.name}` — wrong `#`
prefix for DMs (should be `@`), and for DMs with no name showed
`Message # ` (blank). Now uses the same `roomPrefix(room.type) +
roomDisplayName(room)` pattern as RoomList and MessageArea's
header, so channels render `Message # frontend` and DMs render
`Message @ bob` (or `Message @ (DM)` as fallback).
Tests: 7 new (3 CreateRoomDialog wait/cancel cases, 2 MemberRoster
stale-fetch guards, 5 MessageInput placeholder cases — refactored to
a new MessageInput.test.jsx). 242 pass total; vite build clean.
Spec updated inline at §Layer 4 — Wait for subscription.update — to
describe the timeout-shows-error behavior accurately. No separate
bug-fix section; the new design IS the documented behavior.
…sageInput placeholder Brings the spec in sync with the as-shipped behavior added in the two follow-up commits: 3adde76 feat: expand org rows in MemberRoster to show member list inline 4ece084 fix: timeout-error in CreateRoomDialog + fetch race guards + correct MessageInput placeholder Layer 5 (MemberRoster) gains two subsections: * Inline org expansion — describes the click-to-expand chevron, the three keyed state maps (expandedOrgs / orgChildren cache / orgFetchState), the chat.user.{a}.request.orgs.{orgId}.members one-phase RPC the expansion calls, and the no-buttons-on-children rule (parent org row owns the lifecycle). Loading / error rows documented. * Generation-counter guards — explains memberListGenRef and orgFetchGenRef.current[orgId], the room-switch-mid-fetch race and the rapid-expand-collapse-reexpand race they prevent. Layer 7 (per-room toolbar / badge) gains a one-paragraph note about MessageInput's placeholder using roomPrefix + roomDisplayName, so DMs render `Message @ bob` (with the (DM) placeholder fallback) instead of the misleading `Message #bob-dm`. Owner-viewer ASCII sketch updates the Engineering row to show the collapsed chevron prefix, and a new expanded-view sketch immediately follows it. File inventory: * subjects.js: orgMembers added to the comment. * MessageInput.jsx / MessageInput.test.jsx (new) added. * MemberRoster.jsx comment widened to mention inline org expansion and the gen-counter guards. * styles/index.css comment widened to mention .roster-row-org / .roster-org-children / .roster-chevron. Test count bumped 228 → 242 across 24 → 25 files. No separate "Bug fix" section per the directive that fixes shipped with new code are covered by the spec's design description itself.
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds a two‑phase NATS async-job helper and integrates it across room creation and member-management flows; introduces MemberPicker and MemberRoster UIs, moves the members badge into MessageArea, normalizes encrypted-only messages, updates subjects/constants, adds live smoke tests, and expands unit/integration tests. ChangesAsync job infrastructure & room-create integration
Member management redesign (roster + add picker)
UI integration, formatting, events, and styling
Hooks, helpers, and housekeeping
sequenceDiagram
participant Client as Client<br/>(CreateRoomDialog)
participant NATS as NATS<br/>(PubSub)
participant Server as Server<br/>(room-service)
rect rgba(100, 150, 200, 0.5)
Note over Client,Server: Phase 1: Sync Request/Reply
Client->>NATS: subscribe(user.response.{requestId}) max:1
Client->>NATS: publish(room.create) with X-Request-ID header
NATS->>Server: deliver request
Server->>NATS: sync reply (accepted / error)
NATS->>Client: sync reply received
end
rect rgba(150, 200, 100, 0.5)
Note over Client,Server: Phase 2: Async Result or DM-dedup
alt DM already exists (treatAsSuccess)
Client->>Client: treatAsSuccess(sync) → resolve async:null
else Background success
Server->>NATS: publish(user.response.{requestId}) async result
NATS->>Client: async message (subscription max:1)
Client->>Client: resolve with async result
else Timeout
Client->>Client: Promise.race → AsyncTimeout error
end
end
rect rgba(200, 150, 100, 0.5)
Note over Client: Dialog confirmation flow
Client->>Client: await room presence in summaries
Client->>Client: on observed room → close dialog & call onCreated()
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~65 minutes Possibly related PRs
Suggested reviewers
✨ Finishing Touches🧪 Generate unit tests (beta)
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
chat-frontend/src/lib/roomEventsReducer.js (1)
151-153:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUse normalized message timestamp as
lastMsgAtfallback.On Line 151 and Line 183,
lastMsgAtonly usesevt.lastMsgAt. For synthesized encrypted messages, this can leave room ordering stale whenevt.lastMsgAtis absent even thoughmsg.createdAtexists.Suggested fix
+ const effectiveLastMsgAt = evt.lastMsgAt ?? msg.createdAt ?? prev.lastMsgAt const nextRoomState = { ...prev, pendingLiveMessages, - lastMsgAt: evt.lastMsgAt ?? prev.lastMsgAt, + lastMsgAt: effectiveLastMsgAt, lastMsgId: evt.lastMsgId ?? prev.lastMsgId, unreadCount: isActive ? prev.unreadCount : prev.unreadCount + 1, hasMention: isActive ? false : prev.hasMention || !!evt.hasMention, mentionAll: isActive ? false : prev.mentionAll || !!evt.mentionAll, } ... + const effectiveLastMsgAt = evt.lastMsgAt ?? msg.createdAt ?? prev.lastMsgAt const nextRoomState = { ...prev, messages, - lastMsgAt: evt.lastMsgAt ?? prev.lastMsgAt, + lastMsgAt: effectiveLastMsgAt, lastMsgId: evt.lastMsgId ?? prev.lastMsgId, unreadCount: isActive ? prev.unreadCount : prev.unreadCount + 1, hasMention: isActive ? false : prev.hasMention || !!evt.hasMention, mentionAll: isActive ? false : prev.mentionAll || !!evt.mentionAll, }Also applies to: 183-184
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@chat-frontend/src/lib/roomEventsReducer.js` around lines 151 - 153, The reducer is only falling back to evt.lastMsgAt for lastMsgAt, which misses synthesized encrypted messages that have msg.createdAt; update the assignment for lastMsgAt in the roomEventsReducer (where lastMsgAt, lastMsgId, unreadCount are set) to use a normalized timestamp from evt.msg.createdAt when evt.lastMsgAt is undefined — e.g., compute a fallback like normalizeTimestamp(evt.msg?.createdAt) and use evt.lastMsgAt ?? normalizedMsgTs ?? prev.lastMsgAt; ensure the same change is applied to the other occurrence around lines 183–184 so room ordering updates correctly.
🧹 Nitpick comments (1)
chat-frontend/src/components/manageMembers/AddMembersForm.test.jsx (1)
31-40: ⚡ Quick winAvoid fixed sleeps in tests.
Line 39 introduces a timing dependency that can make this test flaky/slower; this assertion can be synchronous (or
waitFor) without a hard delay.Suggested fix
- it('submit is always clickable; clicking on an empty form is a no-op', async () => { + it('submit is always clickable; clicking on an empty form is a no-op', () => { ... fireEvent.click(submit) - await new Promise((r) => setTimeout(r, 30)) expect(requestWithAsyncResult).not.toHaveBeenCalled() })🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@chat-frontend/src/components/manageMembers/AddMembersForm.test.jsx` around lines 31 - 40, The test uses a fixed sleep (new Promise(... setTimeout 30)) after fireEvent.click which makes it flaky; replace that hard delay with a proper async wait such as waitFor to observe the result of clicking the submit button. Specifically, in AddMembersForm.test.jsx update the test that references requestWithAsyncResult, submit and fireEvent.click to remove the setTimeout and instead await waitFor(() => expect(requestWithAsyncResult).not.toHaveBeenCalled()) (or an equivalent flush/wait helper) so the assertion runs reliably without a fixed sleep.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@chat-frontend/src/components/CreateRoomDialog.jsx`:
- Around line 119-120: The overlay's onClick currently always calls onClose
allowing the dialog to close while a create request is loading; update the
overlay click handler and the Cancel button handler inside CreateRoomDialog
(reference: the outer div with className "dialog-overlay" and the Cancel button
click handler around the Cancel label at/near lines 152-154) to first check the
component's loading state and only call onClose when loading is false (i.e., if
(loading) return/no-op; else call onClose). Ensure the inner dialog
stopPropagation remains intact and keep the disabled styling/behavior consistent
when loading is true.
In `@chat-frontend/src/components/manageMembers/MemberRoster.jsx`:
- Around line 8-68: The code dereferences user.account unconditionally in
hooks/callbacks (e.g., fetchMembers, useCallback deps, isCurrentUserOwner, and
the handlers toggleOrg/handleLeave), which can crash if user is null; fix by
guarding or substituting the account value (use user?.account or const account =
user?.account || null) before use, early-returning from async actions when
account is missing, and update dependent arrays to reference the guarded value
(e.g., replace user.account with account or user?.account in fetchMembers'
dependency list and in useMemo for isCurrentUserOwner) so no hook or callback
reads account from a null user. Ensure generation/ref logic (memberListGenRef,
orgFetchGenRef) continues to operate but only runs when account is present.
In `@chat-frontend/src/components/manageMembers/MemberRoster.test.jsx`:
- Around line 458-461: The mock for request in MemberRoster.test.jsx incorrectly
checks request.mock.calls.length === 1, so the intended first org-members fetch
is skipped; update the condition inside the mockImplementation (the branch
checking subject.includes('.orgs.') && subject.endsWith('.members')) to use
request.mock.calls.length === 0 (or another explicit flag) so the very first
org-members call returns firstFetch and later calls return
Promise.resolve(secondPayload); this ensures the pending/stale path is exercised
as intended.
In `@chat-frontend/src/components/MemberPicker.jsx`:
- Around line 184-185: The dropdown is considered open via hasDropdown even when
disabled, allowing mouse clicks to select results; update the hasDropdown
expression to include !disabled and add guards in the result-item click handler
(the function that selects a member, e.g., selectMember / onResultClick) so it
returns early when disabled; also prevent pointer interactions on result items
when disabled (or skip attaching onClick) and mirror the same guard for keyboard
selection handlers to fully disable selection while disabled.
- Around line 236-241: The ArrowUp/ArrowDown handlers currently call
e.preventDefault() and update setActiveIdx even when no suggestions are visible,
which blocks native caret movement; update the handlers to first check whether
the suggestions dropdown is open (e.g., results.length > 0 or your isOpen flag)
and only then call e.preventDefault() and setActiveIdx; otherwise do nothing so
the native cursor navigation proceeds. Use the existing symbols (e.key
'ArrowDown'/'ArrowUp', setActiveIdx, results, and any isOpen state) to gate the
behavior.
In `@chat-frontend/src/components/RoomMembersBadge.jsx`:
- Around line 18-38: Guard against a null/undefined user before dereferencing
user.account in the RoomMembersBadge effect: update the effect to early-return
when user is falsy (e.g., if (!isChannel || !user) return) and/or replace
user.account in the dependency array with user?.account; also use optional
chaining when calling request (e.g., request(memberList(user?.account, ...)) or
avoid calling request when user is missing) so the effect setup and request
won't throw if auth is briefly null. Ensure you still reset state
(setCount/setErrored) only when appropriate and keep cancelled logic intact.
In `@chat-frontend/src/context/RoomEventsContext.jsx`:
- Around line 44-53: logEvent currently emits potentially sensitive
subject/account values and always logs on hot paths; add an explicit debug flag
(e.g., debugEvents or enableRoomEventDebug) in the RoomEventsContext and
early-return from logEvent when it's false, and replace raw subject/account
values with a sanitized form (redact or short-hash) before logging; apply the
same gating/sanitization to the other console.debug call around lines 66–68 so
no unmasked identifiers are emitted unless the debug flag is enabled. Ensure you
reference the logEvent function and the other debug call site, add a single
source boolean flag in the context/provider, and transform subject/account into
a non-identifying token when logging.
In `@chat-frontend/src/lib/roomFormat.js`:
- Around line 2-3: The searchRoomPrefix helper is inconsistent with roomPrefix:
roomPrefix treats type 'botDM' like 'dm' and returns '@ ', but searchRoomPrefix
only checks for 'dm' and will return '# ' for 'botDM'; update searchRoomPrefix
to include 'botDM' in its DM check (the same way roomPrefix does) so both
functions return '@ ' for 'botDM' types (searchRoomPrefix and roomPrefix are the
unique identifiers to change).
In `@chat-frontend/src/lib/useDebouncedSearch.js`:
- Around line 33-46: The debounce fetch can return out-of-order and overwrite
newer state; add a sequence guard using a ref counter (e.g., requestSeqRef) that
you increment whenever scheduling a fetch (and also increment when reset() is
called to invalidate in-flight responses), capture the current seq in the
timeout closure, and only call setResults(resp ?? []) if the captured seq equals
requestSeqRef.current; keep clearing debounceRef.current as before and ensure
reset() increments the seq/ref to prevent stale fetches from updating state.
In `@docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md`:
- Around line 944-945: Replace the hardcoded test summary string "Tests: 242
pass across 25 files. `vite build` produces a clean production bundle." with a
timeless, non-numeric statement such as "Tests pass in CI/local run and `vite
build` produces a clean production bundle." so the spec doesn't contain stale
numeric counts; locate the exact line containing that string in the document and
update it accordingly.
- Line 104: Several fenced code blocks in the document are missing language
identifiers which triggers markdownlint MD040; update each triple-backtick fence
to include an appropriate language tag (for example replace ``` with ```js,
```jsx, ```ts, or ```text as applicable) so code samples are lint-clean — scan
the file for all occurrences of bare ``` fences (including the ones mentioned in
the review) and add the correct language token for each block based on the
snippet content.
---
Outside diff comments:
In `@chat-frontend/src/lib/roomEventsReducer.js`:
- Around line 151-153: The reducer is only falling back to evt.lastMsgAt for
lastMsgAt, which misses synthesized encrypted messages that have msg.createdAt;
update the assignment for lastMsgAt in the roomEventsReducer (where lastMsgAt,
lastMsgId, unreadCount are set) to use a normalized timestamp from
evt.msg.createdAt when evt.lastMsgAt is undefined — e.g., compute a fallback
like normalizeTimestamp(evt.msg?.createdAt) and use evt.lastMsgAt ??
normalizedMsgTs ?? prev.lastMsgAt; ensure the same change is applied to the
other occurrence around lines 183–184 so room ordering updates correctly.
---
Nitpick comments:
In `@chat-frontend/src/components/manageMembers/AddMembersForm.test.jsx`:
- Around line 31-40: The test uses a fixed sleep (new Promise(... setTimeout
30)) after fireEvent.click which makes it flaky; replace that hard delay with a
proper async wait such as waitFor to observe the result of clicking the submit
button. Specifically, in AddMembersForm.test.jsx update the test that references
requestWithAsyncResult, submit and fireEvent.click to remove the setTimeout and
instead await waitFor(() =>
expect(requestWithAsyncResult).not.toHaveBeenCalled()) (or an equivalent
flush/wait helper) so the assertion runs reliably without a fixed sleep.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b08d7280-ff88-448b-a53c-b6db7431d7a8
📒 Files selected for processing (44)
chat-frontend/scripts/asyncJob.smoke.mjschat-frontend/scripts/liveStack.smoke.mjschat-frontend/src/components/CreateRoomDialog.jsxchat-frontend/src/components/CreateRoomDialog.test.jsxchat-frontend/src/components/ManageMembersDialog.jsxchat-frontend/src/components/ManageMembersDialog.test.jsxchat-frontend/src/components/MemberPicker.jsxchat-frontend/src/components/MemberPicker.test.jsxchat-frontend/src/components/MessageArea.jsxchat-frontend/src/components/MessageArea.test.jsxchat-frontend/src/components/MessageInput.jsxchat-frontend/src/components/MessageInput.test.jsxchat-frontend/src/components/RoomList.jsxchat-frontend/src/components/RoomMembersBadge.jsxchat-frontend/src/components/RoomMembersBadge.test.jsxchat-frontend/src/components/manageMembers/AddMembersForm.jsxchat-frontend/src/components/manageMembers/AddMembersForm.test.jsxchat-frontend/src/components/manageMembers/MemberRoster.jsxchat-frontend/src/components/manageMembers/MemberRoster.test.jsxchat-frontend/src/components/manageMembers/RemoveMemberForm.jsxchat-frontend/src/components/manageMembers/RemoveMemberForm.test.jsxchat-frontend/src/components/manageMembers/RemoveOrgForm.jsxchat-frontend/src/components/manageMembers/RemoveOrgForm.test.jsxchat-frontend/src/components/manageMembers/RoleUpdateForm.jsxchat-frontend/src/components/manageMembers/RoleUpdateForm.test.jsxchat-frontend/src/context/NatsContext.jsxchat-frontend/src/context/RoomEventsContext.jsxchat-frontend/src/lib/asyncJob.jschat-frontend/src/lib/asyncJob.test.jschat-frontend/src/lib/constants.jschat-frontend/src/lib/parseList.jschat-frontend/src/lib/parseList.test.jschat-frontend/src/lib/roomEventsReducer.jschat-frontend/src/lib/roomEventsReducer.test.jschat-frontend/src/lib/roomFormat.jschat-frontend/src/lib/roomFormat.test.jschat-frontend/src/lib/subjects.jschat-frontend/src/lib/subjects.test.jschat-frontend/src/lib/useDebouncedSearch.jschat-frontend/src/lib/useDebouncedSearch.test.jschat-frontend/src/pages/ChatPage.jsxchat-frontend/src/pages/ChatPage.test.jsxchat-frontend/src/styles/index.cssdocs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md
💤 Files with no reviewable changes (8)
- chat-frontend/src/components/manageMembers/RemoveOrgForm.jsx
- chat-frontend/src/components/manageMembers/RemoveMemberForm.jsx
- chat-frontend/src/components/manageMembers/RoleUpdateForm.jsx
- chat-frontend/src/components/manageMembers/RoleUpdateForm.test.jsx
- chat-frontend/src/lib/parseList.test.js
- chat-frontend/src/lib/parseList.js
- chat-frontend/src/components/manageMembers/RemoveOrgForm.test.jsx
- chat-frontend/src/components/manageMembers/RemoveMemberForm.test.jsx
GITMateuszCharczuk
approved these changes
May 13, 2026
- CreateRoomDialog: gate overlay-click dismiss on `loading` so users can't cancel a mid-flight create via outside-click. - MemberRoster / RoomMembersBadge: guard `user?.account` so transient null auth context doesn't crash render/effect setup. - MemberRoster.test: use an explicit `orgFetchCount` counter — the prior `request.mock.calls.length === 1` check was off-by-one (member.list consumes call #1) so the pending/stale path wasn't actually exercised. - MemberPicker: include `!disabled` in `hasDropdown`, guard the result-item click handler when disabled, and gate ArrowUp/ArrowDown preventDefault + activeIdx updates on `hasDropdown` so native caret movement works when no dropdown is open. - useDebouncedSearch: sequence guard (`seqRef`) drops stale fetch resolutions so an older request can't overwrite newer state; reset() also bumps the counter to invalidate in-flight responses. - roomEventsReducer: `lastMsgAt` falls back to `msg.createdAt` when `evt.lastMsgAt` is missing, so synthesized encrypted-message placeholders still advance room ordering. - roomFormat: `searchRoomPrefix` treats `botDM` like `dm` (matches `roomPrefix`), so sidebar and search rows show the same prefix. - AddMembersForm.test: drop fixed 30ms sleep; empty-form submit handler early-returns synchronously so the assertion is reliable without it. - Remove diagnostic console.debug instrumentation that was added for debugging only (roomEventsReducer + RoomEventsContext). - Spec doc: add `text` language identifier to bare ``` fences (markdownlint MD040) and replace the hardcoded "242 tests pass" count with timeless wording.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md`:
- Around line 620-633: The doc claims about debug instrumentation are stale:
update or remove the passage about RoomEventsContext logging (mentions dmSub
callback `chat.user.{account}.event.room`, channel-sub
`chat.room.{roomId}.event`, `openChannelSub` registration, and reducer drop logs
including `subject`, `evt.type`, `roomId`, `lastMsgId`, `hasMessage`,
`hasEncrypted`) to reflect the current implementation; either add the missing
console.debug calls and diagnostic fields to the RoomEventsContext code paths
(in functions/methods named dmSub, openChannelSub, and the reducer drop handler)
so logs contain the listed fields, or change the doc text to state that such
debug-level instrumentation is not present and list what is actually logged
today. Ensure the doc references (`RoomEventsContext`, dmSub, openChannelSub,
reducer) match the code symbols so future readers can correlate the text with
the implementation.
- Line 556: The inline code span contains a trailing space (`Message # `) that
triggers markdownlint MD038; edit the span so it has no interior trailing space
(change `Message # ` to `Message #`) or otherwise represent the intended text
without the trailing whitespace, ensuring the snippet in the document (the
offending `Message # ` span) is updated accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 867052b2-721c-451e-bf56-205f36054ab6
📒 Files selected for processing (11)
chat-frontend/src/components/CreateRoomDialog.jsxchat-frontend/src/components/MemberPicker.jsxchat-frontend/src/components/RoomMembersBadge.jsxchat-frontend/src/components/manageMembers/AddMembersForm.test.jsxchat-frontend/src/components/manageMembers/MemberRoster.jsxchat-frontend/src/components/manageMembers/MemberRoster.test.jsxchat-frontend/src/context/RoomEventsContext.jsxchat-frontend/src/lib/roomEventsReducer.jschat-frontend/src/lib/roomFormat.jschat-frontend/src/lib/useDebouncedSearch.jsdocs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md
🚧 Files skipped from review as they are similar to previous changes (7)
- chat-frontend/src/components/CreateRoomDialog.jsx
- chat-frontend/src/components/manageMembers/MemberRoster.jsx
- chat-frontend/src/components/manageMembers/AddMembersForm.test.jsx
- chat-frontend/src/components/MemberPicker.jsx
- chat-frontend/src/components/RoomMembersBadge.jsx
- chat-frontend/src/lib/useDebouncedSearch.js
- chat-frontend/src/components/manageMembers/MemberRoster.test.jsx
…038) - Drop the "Diagnostic logging" subsection — the console.debug instrumentation it described was removed in the previous commit. - Update the inline code snippet to match the simplified reducer (direct `return state` on missing lastMsgId, no debug log). - Update the file-tree comment for `RoomEventsContext.jsx` to drop the stale "diagnostic console.debug logs" descriptor. - Drop the secondary `Message # ` example to fix markdownlint MD038 (spaces inside code spans); `Message #bob-dm` already illustrates the bug the new placeholder helper avoids.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md (1)
927-928: 💤 Low valueSimplify the redundant "Tests:" label.
The phrasing "Tests: Tests pass..." reads awkwardly with the duplicate word. Consider one of these alternatives:
- "All tests pass in CI/local runs and..."
- "Tests and build: Tests pass in CI/local runs and..."
This makes the sentence flow more naturally.
📝 Suggested alternatives
-**Tests:** Tests pass in CI/local runs and `vite build` produces a clean -production bundle. +All tests pass in CI/local runs and `vite build` produces a clean production bundle.or
-**Tests:** Tests pass in CI/local runs and `vite build` produces a clean -production bundle. +**Tests and build:** Tests pass in CI/local runs and `vite build` produces a clean production bundle.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md` around lines 927 - 928, The sentence currently reads "**Tests:** Tests pass in CI/local runs and `vite build` produces a clean production bundle." — remove the duplicated "Tests" label and rephrase for clarity; for example replace the line with "All tests pass in CI/local runs and `vite build` produces a clean production bundle." or use "**Tests and build:** Tests pass in CI/local runs and `vite build` produces a clean production bundle." to keep a label and avoid redundancy.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md`:
- Around line 927-928: The sentence currently reads "**Tests:** Tests pass in
CI/local runs and `vite build` produces a clean production bundle." — remove the
duplicated "Tests" label and rephrase for clarity; for example replace the line
with "All tests pass in CI/local runs and `vite build` produces a clean
production bundle." or use "**Tests and build:** Tests pass in CI/local runs and
`vite build` produces a clean production bundle." to keep a label and avoid
redundancy.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d4abb2f-7c39-428e-a7c5-d942cc5e2a5e
📒 Files selected for processing (1)
docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md
"**Tests:** Tests pass..." read awkwardly with the duplicated word.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md`:
- Line 17: Replace the hardcoded retrospective sentence "Ten commits, all in
`chat-frontend/`." with a timeless phrasing that doesn't depend on commit count,
e.g., "Implemented in `chat-frontend/` as part of this overhaul PR" (or
similar), so the summary remains accurate after rebases/squashes; update the
exact string occurrence in the document (the line containing "Ten commits, all
in `chat-frontend/`.") accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 23a8ce6a-dfff-4c03-bd51-46ef927b75f2
📒 Files selected for processing (1)
docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md
Will drift on rebase/squash. Same flavor as the prior "242 tests" fix.
GITMateuszCharczuk
pushed a commit
that referenced
this pull request
May 14, 2026
…ssage curry Three rebase-driven regressions in the plan: 1. Task 2.5's slimmed ChatPage was about to drop the upstream RoomMembersBadge (PR #178) and revert to a manual "{userCount} members" + Members button. Restored: - Imports RoomMembersBadge + roomDisplayName. - membersRefreshKey state + bump-on-dialog-close pattern. - Header strip uses RoomMembersBadge with refreshKey; LeaveRoomButton remains. - Title now uses roomDisplayName(selectedRoom) so DMs render. - Interim MessageArea continues to receive onOpenMembers + membersRefreshKey until Task 3.6 swaps in RoomMessageArea. 2. Task 3.6's RoomMessageArea was passing two args to jumpToMessage. The per-room useRoomEvents(roomId).jumpToMessage is curried — takes only (messageId). Fixed call site and added a clarifying note that the two-arg form lives on useRoomSummaries() for cross-room jumps. 3. RoomMessageArea now forwards `room` to MessageList so the per-row read-receipt kebab can construct its NATS subject (the A3 plumbing terminus). https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
GITMateuszCharczuk
added a commit
that referenced
this pull request
May 15, 2026
* docs: add chat-frontend thread side-panel design
Adds the design for a right-hand thread panel in chat-frontend:
- Layout refactor into AppHeader / Sidebar / ChatPage / ThreadRightBar
- New ThreadEventsContext + reducer mirroring RoomEventsContext
- Hover MessageActions with Thread (open panel) and Reply (quote-reply
via quotedParentMessageId, context-aware routing)
- "Also send to channel" checkbox on thread input mapped to tshow flag
- Main-feed filtering: thread replies hidden unless tshow=true, parent
tcount/lastReplyAt bumped client-side regardless
- Presentational MessageList / MessageInputForm with thin Room/Thread
containers (Approach A)
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: simplify thread spec — no main-feed filtering, defer live thread events
Backend already filters at the boundary: main-feed APIs (history,
broadcast) only return top-level messages plus tshow:true thread
replies. msg.thread and per-message lookups are the only paths that
return arbitrary thread replies.
Spec updates:
- Drop client-side filtering of thread replies; main reducer appends
unconditionally.
- Parent tcount/lastReplyAt bumps from the single observed case
(tshow:true reply arrives in main feed).
- Live delivery of other users' thread replies deferred to a separate
events ticket; ThreadEventsContext loses its room-event subscription
and gains optimistic append (REPLY_SENT_LOCAL) for the user's own
replies.
- REPLY_RECEIVED action replaced with REPLY_SENT_LOCAL.
- Tests and edge cases aligned with the new contract.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: lock thread spec — quote-reply UX, edit/delete hover, panel layout
Captures all decisions from the design review round:
- Hover actions = Thread + Reply (quote) + Edit + Delete; Edit inline
(Enter saves, Esc cancels), Delete via confirm dialog. Available on
main feed, parent inside thread, thread replies.
- Quote-reply rendering: single shared QuotedBlock used both as the
staging chip above the input (sender top, one-line content +
right-justified ✕) and in-bubble above the reply content (shared
message background, click-to-jump-to-original via focusMessageId).
- Reply routing rule (4 cases). Disabled+tooltip when quoting a
tshow:true thread reply from the main feed (gatekeeper boundary).
- Thread panel: chronological list (no sticky parent), 380px wide,
empty-state line "No replies yet — be the first to reply", input
placeholder "Reply…".
- "Also send to channel" checkbox: defaults off, resets after every
successful send.
- Failed sends: keep optimistic row tagged failed with ⟳ retry; new
reducer actions REPLY_SEND_FAILED / REPLY_RETRIED.
- Cross-context tshow pickup: thread context observes main
MESSAGE_RECEIVED, appends matching tshow:true replies via
TSHOW_OBSERVED. Main reducer's THREAD_REPLY_OBSERVED is deduped by
per-parent observed-id Set.
- Layout split locked: AppHeader (search/theme/user/logout), room-
header (room name + Members + Leave), Sidebar (rooms + create).
- Drop "lastReplyAt" from the badge (field not on Message payload).
- Open questions cleared.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread spec — drop tshow, address reviewer findings
Three parallel reviewers found a critical backend gap and multiple
frontend/UX gaps:
- Backend reviewer: tshow is NOT wired through (no field on
SendMessageRequest, gatekeeper drops it, broadcast-worker doesn't
filter). The "Also send to channel" feature can't ship without
backend work.
- Frontend reviewer: mergeById not extracted; cross-context observer
has no machinery in RoomEventsContext; msgThread/msgEdit/msgDelete
subject builders missing.
- UX reviewer: failed-reply dismiss missing; loading/error UI for
thread panel undefined; deleted-quoted-message placeholder
unspecified; keyboard accessibility entirely missing.
Spec changes:
- Drop "Also send to channel" / tshow checkbox from v1 entirely.
Add as future work; needs paired backend ticket.
- Add client-side filter: roomEventsReducer.MESSAGE_RECEIVED drops
messages whose threadParentMessageId is set (broadcast-worker
publishes them unconditionally today).
- Drop TSHOW_OBSERVED action and cross-context observer. Replace
with OWN_THREAD_REPLY_SENT — only the sender's own reply bumps
parent tcount in real time; other users' replies update on next
history reload.
- Add REPLY_DISMISSED action + ✕ dismiss button on failed rows.
- Add HISTORY_LOADING action; explicit loading/error/empty states
in ThreadRightBar; "Try again" on history failure.
- New shared util: src/lib/messageBuffer.js (appendBounded +
mergeById preserving _local / _status markers).
- New subject builders: msgThread, msgEdit, msgDelete in subjects.js
(edit/delete exist server-side but were missing on the frontend).
- New "Keyboard & accessibility" section: tabindex, :focus-within,
Esc behavior, focus management, aria-live, aria-labels.
- QuotedBlock: deleted-snapshot placeholder; plain-text only (no
markdown); chip stays during in-flight publish.
- Cross-room main-input quote: cleared on room switch.
- Remove obsolete "tshow reply in main feed" routing case.
Spec now decision-complete: no open questions, three small Red-phase
verification items only.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: document verified broadcast-worker pipeline + deferred filter
Traced the actual pipeline: client → gatekeeper → MESSAGES_CANONICAL
→ broadcast-worker (direct consumer, no FANOUT despite README).
broadcast-worker has no thread-parent check in publishChannelEvent /
publishDMEvents. Frontend filter stays as the v1 workaround; the
proper server-side fix is paired with the tshow ticket.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.1 — foundation utilities
Chapter 1 of the implementation plan: messageBuffer extraction,
roomEventsReducer refactor to use it, and new subject builders
(msgThread / msgEdit / msgDelete).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.2 — layout shell
Chapter 2: extract AppHeader, Sidebar, MainApp; slim ChatPage to
middle column with its own room-header strip; rename top-level CSS
selectors.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.3 — message component refactor
Chapter 3: extract presentational components (QuotedBlock,
MessageActions, MessageRow, MessageList, MessageInputForm) plus
Room-scoped containers (RoomMessageArea, RoomMessageInput); wire
ChatPage to use the new containers; delete old MessageArea/Input.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.4 — edit + delete actions
Chapter 4: Red-phase verification of msg.edit/msg.delete payloads;
MESSAGE_EDITED_LOCAL / MESSAGE_DELETED_LOCAL reducer actions; Edit
and Delete buttons in MessageActions (own messages only); inline
edit mode on MessageRow; DeleteConfirmDialog; RPC wiring in
RoomMessageArea; expose dispatch on RoomEventsContext.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.5 — quote-reply staging + jump-to-original
Chapter 5: lift quotedTarget to ChatPage, wire Reply icon → chip
→ publish payload; clear on room switch; E2E test for the full
chain; E2E test for click-to-jump on in-bubble QuotedBlock.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.6 — thread state plumbing
Chapter 6: threadEventsReducer (OPEN/CLOSE/HISTORY/REPLY actions
including optimistic _local + failed/retried/dismissed); ThreadEventsContext with race-discard openThread, sendReply, retryReply,
dismissReply; mount the provider under RoomEventsProvider in App.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.7 — thread panel UI
Chapter 7: ThreadMessageInput, ThreadMessageArea (renders parent
+ replies, surfaces retry/dismiss), MessageList parent-row context
override, MessageRow failed-row UI (⟳ + ✕), ThreadRightBar shell,
MainApp mount + ChatPage wiring of the Thread hover icon, and CSS
for the panel layout.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.8 — cross-context wiring + filter + tcount badge
Chapter 8: filter thread replies from main feed live broadcast;
OWN_THREAD_REPLY_SENT action for parent tcount bump; cross-dispatch
from ThreadEventsContext on send success; reply-count badge on
MessageRow; close thread on room switch.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: thread plan ch.9 — a11y, mutual exclusion, final smoke
Chapter 9: InRoomSearch ↔ ThreadRightBar mutual exclusion; Esc
closes the thread panel; focus management on open/close; logout
reset verification; final integration smoke checklist; spec↔plan
coverage table.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix B3 — CSS descendant rule renames in Ch.2 task 2.7
Reviewer noted that .chat-main has descendant rules at lines 536/543
that the original rename instruction missed; explicit step added to
catch these.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix B5 — move dispatch-exposure to Task 4.1.5 (prerequisite)
Task 4.6 used useRoomEvents().dispatch before Task 4.7 wired it.
Moved the prerequisite to Task 4.1.5 so the impl path works
end-to-end in dev, not just in test mocks.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix N3 — test the "(edited)" marker explicitly
The marker was added beyond spec for standard UX, but had no test.
Two tests added: marker renders when editedAt is set, omitted when
not.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix N2 — clarify mock replacement in Ch.4 task 4.6
The original wording "append a publish + dispatch mock" was
ambiguous; vitest hoists both vi.mock calls and the second silently
wins. Explicit REPLACE instruction added with a re-run gate to catch
earlier-test breakage immediately.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix N1 — add HISTORY_LOADING reducer tests
The action existed in the reducer but had no test. Two tests added:
flips historyLoading=true for active parent; no-op for non-active.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix B1 — publish is sync void, not async
Verified at chat-frontend/src/context/NatsContext.jsx:74-78: publish
returns undefined and throws synchronously if not connected. Plan
previously awaited it and used mockResolvedValue/mockRejectedValue,
which would never trigger the failure path.
Rewired:
- ThreadEventsContext.publishReply: sync, throws if no active thread
- ThreadEventsContext.sendReply / retryReply: try/catch around sync
publish, dispatch REPLY_SEND_FAILED on sync throw
- Tests: replace mockResolvedValue/mockRejectedValue with
mockImplementation(() => { throw new Error(...) }) for failure
scenarios; remove awaits where not needed
- ThreadMessageInput.handleSubmit: sync; quote chip clears after the
sync sendReply call (failures will surface via the _local row's
_status='failed', not via promise rejection)
Caveat documented: no broker-ack signal in v1, so failures rejected
asynchronously by the broker are invisible to the client until the
thread is reopened.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix B4 — vi.mock must be hoisted, not inside it()
Reviewer caught vi.mock('./MessageRow', …) inside an it() block in
Ch.7 task 7.3 step 2. vitest hoists vi.mock to module scope so the
inner call silently no-ops, leaving the test running against the
real MessageRow (or the prior top-of-file mock from Task 3.4).
Rewrote step 2 to extend the existing top-level mock and append a
clean test that asserts on data-context attributes. Added a re-run
gate to catch breakage of prior tests in the same file.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan fix B2 — wire Edit / Delete inside the thread panel
Reviewer caught that Ch.4 wired Edit/Delete only in RoomMessageArea
(main feed); Ch.7's ThreadMessageArea didn't forward onEdit /
onDelete to MessageList. Spec requires them on thread replies and
on the parent rendered inside the panel.
Two new tasks inserted between 7.2 and 7.3:
- Task 7.2.5: REPLY_EDITED_LOCAL / REPLY_DELETED_LOCAL actions on
threadEventsReducer, with full TDD test coverage.
- Task 7.2.6: ThreadMessageArea now owns editingMessageId +
pendingDelete, publishes msg.edit / msg.delete against the active
parent's room/site, and routes the optimistic dispatch correctly:
- parent edits → roomDispatch MESSAGE_EDITED_LOCAL
- parent deletes → roomDispatch MESSAGE_DELETED_LOCAL
- reply edits → threadDispatch REPLY_EDITED_LOCAL
- reply deletes → threadDispatch REPLY_DELETED_LOCAL
ThreadEventsContext now exposes `dispatch` on its value so the
container can dispatch reducer actions directly.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A1 — refresh NatsContext line refs after rebase
publish() moved from lines 74-78 to 129-133 after upstream
NatsContext additions for asyncJob protocol; behavior unchanged
(still sync void, still throws if not connected) so the underlying
B1 fix stands.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A2 — guard CSS rename against .chat-main-* prefix collision
Post-rebase index.css now has TWO inner-layout helpers
(.chat-main-with-side-panel, .chat-main-content) that share the
prefix with .chat-main but must NOT be renamed. Step 2 of Ch.2 task
2.7 now warns against naive sed s/.chat-main/.chat-page/g and
prescribes a word-boundary-aware grep gate.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A3 — MessageRow keeps the read-receipt kebab
Upstream PR #177 added MessageActionMenu (kebab) on each message
row to expose read receipts. Our plan's MessageRow lifted only
sender/time/content/MessageActions and would have silently dropped
the kebab.
Adjustments:
- Ch.3 task 3.3 prose: clarify MessageActionMenu (kebab) and
MessageActions (Reply/Edit/Delete hover) are separate components
that render side-by-side. Document they have different visibility
rules.
- MessageRow signature now takes `room` and renders
<MessageActionMenu room={room} message={message}/> alongside
<MessageActions/>.
- MessageRow tests mock '../MessageActionMenu' (it depends on
NatsContext) and assert the kebab mounts with the right props.
Test count bumps 5 -> 6.
- MessageList plumbs `room` through to each row.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A4 — preserve upstream RoomMembersBadge + fix jumpToMessage curry
Three rebase-driven regressions in the plan:
1. Task 2.5's slimmed ChatPage was about to drop the upstream
RoomMembersBadge (PR #178) and revert to a manual
"{userCount} members" + Members button. Restored:
- Imports RoomMembersBadge + roomDisplayName.
- membersRefreshKey state + bump-on-dialog-close pattern.
- Header strip uses RoomMembersBadge with refreshKey;
LeaveRoomButton remains.
- Title now uses roomDisplayName(selectedRoom) so DMs render.
- Interim MessageArea continues to receive onOpenMembers +
membersRefreshKey until Task 3.6 swaps in RoomMessageArea.
2. Task 3.6's RoomMessageArea was passing two args to
jumpToMessage. The per-room useRoomEvents(roomId).jumpToMessage
is curried — takes only (messageId). Fixed call site and added
a clarifying note that the two-arg form lives on
useRoomSummaries() for cross-room jumps.
3. RoomMessageArea now forwards `room` to MessageList so the
per-row read-receipt kebab can construct its NATS subject (the
A3 plumbing terminus).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): extract messageBuffer utility (appendBounded, mergeById)
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): restore mergeById to plan's natural algorithm
Task 1.1's implementer flagged a contradiction: the cap test
expected result[0] = new-2, which forced an incoming-reversal hack
that contradicts both the "incoming first, then existing" rule and
the caller convention (incoming = older history page, existing =
newer live tail).
The test was buggy, not the impl. Resolving by:
1. Restoring mergeById to its natural shape (no reverse): build
merged = [...incoming, ...existing-minus-incoming], then cap
from the front so the oldest entries (= incoming history page
beyond the window) are sliced off.
2. Renaming the cap test's incoming ids new-1/new-2 -> hist-1/
hist-2 so it's obvious those are the older entries about to be
evicted; updating the assertions to expect e0 at front and
e199 at end (the natural outcome).
3. Mirroring both fixes back into the plan markdown so the trap
doesn't recur in any later regenerated test/impl.
All 7 messageBuffer tests still pass.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* refactor(chat-frontend): use messageBuffer in roomEventsReducer
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add msgThread, msgEdit, msgDelete subject builders
* feat(chat-frontend): add AppHeader component
Extracts the global header controls (SearchBar, user chip, ThemeToggle,
Logout) into a standalone AppHeader component with full test coverage.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add Sidebar component
* feat(chat-frontend): add MainApp shell
Adds MainApp component with lifted selectedRoom/searchQuery state,
rendering AppHeader + Sidebar + (SearchResultsPane|ChatPage) with
full TDD cycle (4 tests, red→green).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): keep MainApp evict-effect plan-faithful, populate test mock instead
Task 2.3's implementer added a 'summaries.length > 0' guard to the
"clear selection if room disappears" effect so that mid-test
clicks didn't trigger the eviction during their empty-summaries
mock. The guard worked but introduced a real regression: a user
in their last remaining channel who gets kicked would see their
summaries drop to length 0 and the effect would NOT clear the
stale selectedRoom.
Reverting to the plan's effect (matches upstream ChatPage's pattern
at line 30-36) and fixing the test mock to populate summaries with
the rooms the tests click. Mirrored the mock fix into the plan
markdown so future regenerations don't hit the same trap.
4/4 MainApp tests + full chat-frontend suite still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A4.1 — Task 2.6 tests use the new RoomMembersBadge mock
A4 added <RoomMembersBadge> to ChatPage's room-header strip but the
Task 2.6 test mocks were still asserting the old "{N} members" +
Members button shape. Updated:
- New vi.mock for '../components/RoomMembersBadge' renders a
predictable "Members ({userCount})" button + data-* props for
refreshKey verification.
- "channel" test asserts the badge button is present alongside Leave.
- "DM" test asserts Leave is hidden (the badge itself returns null
for DMs in real code; the mock still renders so we focus on Leave).
- "click Members" test fires the badge button.
- "no room" test relaxes regex from ^members$ to ^members to catch
the new button name.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* refactor(chat-frontend): mount MainApp shell from App
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* refactor(chat-frontend): slim ChatPage to middle column + room-header
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* test(chat-frontend): rewrite ChatPage tests for slimmed middle-column shape
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A4.2 — Task 2.6 room-name regex needs the '#' prefix
The /general/ regex matched both the room-name span ('# general')
and the LeaveRoomButton mock's button text ('Leave general'),
breaking the test. Mirroring the implementer's /# general/ fix
into the plan so a future regeneration doesn't re-introduce the
ambiguity.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* style(chat-frontend): rename layout selectors + add room-header strip
Rename page-level CSS selectors to match the new shell classNames
introduced in Tasks 2.1–2.5:
.chat-layout → .app-shell
.chat-header → .app-header
.chat-header-search → .app-header-search
.chat-header-user → .app-header-user
.chat-header-logout → .app-header-logout
.chat-body → .app-row
.chat-sidebar → .app-sidebar
.chat-main → .chat-page
.chat-sidebar .room-list → .app-sidebar .room-list
.chat-main .message-area / .message-list → .chat-page …
Inner-layout helpers (.chat-main-with-side-panel, .chat-main-content)
are unchanged.
Add new rules: .chat-room-header (+ .chat-room-name, .chat-room-header-spacer)
and .chat-page-body.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add QuotedBlock (chip + bubble variants)
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add MessageActions hover menu (Thread + Reply)
* feat(chat-frontend): add MessageRow with embedded QuotedBlock + MessageActions
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add presentational MessageList
* feat(chat-frontend): add presentational MessageInputForm
* docs: plan adj A4.3 — Task 3.6 jumpToMessage assertion uses curried 1-arg form
A4 changed RoomMessageArea to use the per-room jumpToMessage(msgId)
curry, but the Task 3.6 test still asserted the upstream 2-arg
form jumpToMessage('r1', 'a'). The test would have failed against
the corrected component. Reverted assertion to single-arg shape
matching useRoomEvents(roomId).jumpToMessage's signature.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add RoomMessageArea container
Replaces the message-area body from the old MessageArea with a new
container that binds useRoomEvents(roomId) state, drives loadHistory on
mount/room-change, owns live-tail auto-scroll, renders MessageList, and
surfaces the "Jump to latest" pill in HISTORICAL buffer mode.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add RoomMessageInput container
Wraps MessageInputForm with NATS publish side-effect for room message
sending. Publishes msg.send with id/content/requestId payload; includes
quotedParentMessageId when a quotedTarget is staged.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* refactor(chat-frontend): wire new message containers, drop old MessageArea/Input
Replace MessageArea/MessageInput with RoomMessageArea/RoomMessageInput in
ChatPage, update test mocks to match, delete the four legacy component files,
and append hover-reveal CSS (message-row, message-actions, quoted-block).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A5 — Task 4.1 verification + correct edit/delete payload shapes
Task 4.1's "verify backend payload" step was a guess ({ messageId,
createdAt, content, requestId }) marked TODO. Read the actual Go
structs at history-service/internal/models/message.go:60-77:
EditMessageRequest{ MessageID string, NewMsg string }
DeleteMessageRequest{ MessageID string }
Differences from the plan's guess:
- "newMsg" (NOT "content") is the body field — the Go json tag.
- No createdAt in either request.
- No requestId in either request — request-id flows via NATS
headers, not the JSON body.
Adjustments:
- Task 4.1 step 3: documented the verified shapes inline so
future readers don't re-guess.
- Task 4.6 RoomMessageArea impl: handleEditSubmit now publishes
{ messageId, newMsg }; handleDeleteConfirm publishes
{ messageId }. Dropped uuidv4 import (unused).
- Task 4.6 tests: assertions updated to the new payload shapes.
- Task 7.2.6 ThreadMessageArea impl + tests (mirror inside thread
panel): same payload corrections.
The MESSAGE_EDITED_LOCAL / MESSAGE_DELETED_LOCAL reducer actions
are unchanged — they use "content" because that's the local
buffer's field name. Only the network payload changed.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): expose dispatch + useRoomDispatch from RoomEventsContext
* feat(chat-frontend): roomEventsReducer handles MESSAGE_EDITED_LOCAL / DELETED_LOCAL
* feat(chat-frontend): add Edit / Delete to MessageActions (own only)
* docs: plan adj A6 — Task 4.4 MessageRow keeps room prop + read-receipt kebab
Task 4.4's replacement MessageRow.jsx dropped the `room` prop and
the <MessageActionMenu> render that Task 3.3 had added (per
adjustment A3). That regression would silently remove the read-
receipt kebab from every message row.
Restored both:
- Component signature accepts `room` again.
- Render appends <MessageActionMenu message={message} room={room}/>
next to <MessageActions/> in the non-editing state.
- Deleted-row variant continues to NOT render the kebab (the
short-circuit return path).
The Task 3.3 tests assert the kebab is mounted via the
'../MessageActionMenu' mock; without this fix those tests would
re-fail when Task 4.4 ships.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): inline edit + deleted placeholder + (edited) marker on MessageRow
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add DeleteConfirmDialog
Modal dialog with cancel/delete buttons and Esc-to-dismiss behavior.
Used by RoomMessageArea to confirm message deletion.
- Renders confirmation prompt: 'Delete this message? This cannot be undone.'
- Cancel button and Esc key both call onCancel
- Delete button calls onConfirm
- Buttons disabled when pending is true
- Shows 'Deleting…' text while pending
Tests: 4 tests, all passing
* docs: plan adj A6.1 — Task 4.6 preserves room forwarding + curried jumpToMessage
Task 4.6's full-replacement RoomMessageArea.jsx dropped two A4
fixes that Task 3.6 had baked in:
1. <MessageList room={room} ...> forwarding (so MessageRow can
pass room to the read-receipt kebab).
2. The curried per-room jumpToMessage(msgId) call (single arg);
the plan reverted to the upstream 2-arg jumpToMessage(roomId,
msgId) form which doesn't exist on useRoomEvents().
Step 4's MessageList snippet also dropped room={room} from the
MessageRow render — fixed alongside.
Without these, the read-receipt kebab disappears and click-to-
jump errors with "jumpToMessage is not a function".
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): wire msg.edit / msg.delete RPCs in RoomMessageArea
Adds edit/delete state management and RPC dispatch to RoomMessageArea,
forwards new props through MessageList to MessageRow, and adds 6 new
tests covering edit mode, cancel, submit, delete dialog, cancel, and confirm.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A4.4 — Task 5.3 jumpToMessage assertion uses curried 1-arg form
Same trap as A4.3: the per-room useRoomEvents().jumpToMessage is
already bound to roomId, so the click-to-jump path calls it with
just the messageId. The Task 5.3 quoted-block test asserted the
upstream 2-arg form and would have failed.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): wire quote-reply staging in main feed via ChatPage
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* test(chat-frontend): E2E click-to-jump from in-bubble QuotedBlock
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A7 — capture two real-world fixes from Ch.5 execution
Task 5.2 E2E: the file-top vi.mock for RoomMessageInput survives
vi.resetModules() and keeps serving its stub even for dynamic
imports. Override with importActual so the real component loads —
the E2E needs the actual publish wiring.
Task 5.3 click-to-jump: jsdom does not implement scrollIntoView,
which RoomMessageArea's auto-scroll effect calls on mount. Stub
window.HTMLElement.prototype.scrollIntoView = vi.fn() at the top
of the test file.
Both fixes are now in the plan's test snippets.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add threadEventsReducer
Pure reducer owning open-thread state: active parent, buffered replies,
history loading/error, and optimistic-message status flags. Implements
race-discard pattern for parentId-tagged actions and deduplication for
REPLY_SENT_LOCAL. 17 tests covering all actions, short-circuit paths,
and _local row preservation during HISTORY_LOADED merges.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): add ThreadEventsContext
Implements ThreadEventsContext with race-discard generation guard (mirrors
RoomEventsContext), exposes openThread/closeThread/sendReply/retryReply/dismissReply,
and handles sync publish throws with optimistic REPLY_SEND_FAILED dispatch.
8 tests, all passing; full suite 354/354 green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): mount ThreadEventsProvider in App
* feat(chat-frontend): add ThreadMessageInput container
* feat(chat-frontend): add ThreadMessageArea container
Renders the thread parent message first (resolved live from the room
buffer with a thin stub fallback) followed by the reply list, wiring
retry/dismiss back to ThreadEventsContext and auto-scrolling on history
load and reply append.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): threadEventsReducer handles REPLY_EDITED_LOCAL / REPLY_DELETED_LOCAL
* feat(chat-frontend): wire Edit / Delete RPCs inside the thread panel
Mirror RoomMessageArea's edit/delete pattern in ThreadMessageArea: expose
dispatch from ThreadEventsContext, add local editingMessageId/pendingDelete
state, and route edit/delete via the room reducer (MESSAGE_EDITED_LOCAL /
MESSAGE_DELETED_LOCAL) for the parent message and the thread reducer
(REPLY_EDITED_LOCAL / REPLY_DELETED_LOCAL) for replies. Publishes
msg.edit / msg.delete subjects using the active parent's roomId + siteId.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): MessageList honors parentMessageId for thread-parent rows
* feat(chat-frontend): failed-row Retry / Dismiss buttons on MessageRow
* feat(chat-frontend): add ThreadRightBar
* feat(chat-frontend): mount ThreadRightBar in MainApp; wire Thread hover icon
Mount ThreadRightBar as a third column in MainApp when activeParent is set
(via useThreadEvents). Wire ChatPage's onThread prop to call openThread with
the full parent identity (roomId, siteId, messageId, createdAtMs).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* style(chat-frontend): thread panel layout + failed-row indicator
* docs: plan adj A8 — Task 8.3 cross-dispatch uses sync publish pattern
Task 8.3's plan still had `await publishReply` and
`publish.mockRejectedValue` — both incorrect per the verified A1
finding that NatsContext.publish is sync void and throws sync.
Mirroring the Task 6.2 pattern: publishReply is synchronous; the
cross-dispatch fires after it returns successfully (no sync
throw). The failure test uses mockImplementation(() => { throw }).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): filter thread replies out of main feed live broadcast
Drops MESSAGE_RECEIVED events whose msg.threadParentMessageId is non-empty
so broadcast-worker's room-scoped publish doesn't flicker thread replies into
the main feed (Task 8.1).
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): ThreadEventsContext dispatches OWN_THREAD_REPLY_SENT on send success
After publishReply succeeds (no sync throw), cross-dispatches OWN_THREAD_REPLY_SENT
into RoomEventsContext via useRoomDispatch so the parent message tcount bumps
immediately without waiting for a broadcast event (Tasks 8.2 + 8.3).
retryReply gets the same cross-dispatch on retry success.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* docs: plan adj A9 — Task 8.3 sibling describe needs explicit mock reset
The implementer caught a real test-isolation bug: the new
"cross-dispatch OWN_THREAD_REPLY_SENT" describe is a SIBLING of
the outer describe('ThreadEventsContext'), so it does NOT inherit
the outer beforeEach that resets request/publish. Without an
explicit reset, the sync-throw publish.mockImplementation from
the failure test leaks into the next file's success test and
silently routes through the catch branch — making roomDispatch
never fire and the assertion fail mysteriously.
Patched the plan's beforeEach to also reset request + publish,
with a comment explaining why.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): reply-count badge on parent messages
* docs: plan adj A10 — Task 8.5 uses module-scoped-mutable mock pattern
The plan's Task 8.5 test used vi.doMock + dynamic import to vary
activeParent — the same fragile pattern that needed importActual
escapes in Task 5.2. Switching to the module-scoped-let + factory
closure pattern that worked cleanly in Task 7.6 (MainApp tests).
Side benefit: the existing top-level useThreadEvents mock from
Task 7.6 needs to be REPLACED to expose closeThread + a mutable
activeParent. The new shape covers all current and future
ChatPage thread-related tests with a single mock.
Added an extra it() that verifies closeThread is NOT called on
same-room re-render — guards against an over-eager effect that
fires on every render.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): close thread on room switch
* docs: plan adj A11 — Task 9.1 reuses module-scoped mock from A10
Same fix as A10: the Task 9.1 test for "Ctrl-F closes any open
thread" used vi.doMock + dynamic import to vary activeParent.
Replaced with mockActiveParent = {...} + the existing top-level
mock from A10. Cleanup runs in afterEach.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): mutual exclusion InRoomSearch ↔ ThreadRightBar
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): Esc closes the thread panel
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): focus management on thread open/close
Focus the close button when ThreadRightBar mounts; restore focus to the
trigger element when the thread panel closes.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* chore: gitignore vitest cache directories
The .vite/ cache appears at the repo root when vitest is run from
chat-frontend's parent (e.g. from /home/user/chat directly). It's
a transient cache, never source. Added both the root-level and the
chat-frontend-local paths.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): review C1 + C2 — tcount + thread quote-jump
Two Critical findings from the 5-reviewer audit:
C1 (Reviewer 4): retryReply was re-dispatching OWN_THREAD_REPLY_SENT
on every successful retry, on top of the dispatch from the initial
sendReply. Combined they could double-bump the parent's tcount in
scenarios where the row went failed → retried (more importantly,
once broker-acks land and a row can go failed → retried → failed →
retried, every recovery would inflate the badge). Removed the
dispatch from retryReply with a comment explaining the convention:
"a retry is the continuation of a logical send, not a new one".
Stale-by-1 tcount until the next msg.thread reload is acceptable.
C2 (Reviewer 4): ThreadMessageArea never passed onJumpToMessage to
MessageList, so clicking an in-bubble QuotedBlock inside the thread
panel silently no-op'd in QuotedBlock.handleClick. Wired it via
useRoomSummaries().jumpToMessage(activeParent.roomId, msgId) — the
unbound 2-arg form, because the quoted message lives in the parent's
room (which the thread panel's current view doesn't otherwise scope
to).
Both fixes are TDD-driven with new failing tests first.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): review C3 — DeleteConfirmDialog claims Esc via capture phase
Reviewer 5 caught an Esc collision: when the DeleteConfirmDialog
is mounted inside ThreadRightBar, pressing Esc fires BOTH the
dialog's window listener AND the aside's onKeyDown — closing the
dialog and the thread in the same keypress.
Fix: dialog's keydown listener now attaches in the capture phase
(window.addEventListener('keydown', handler, true)) and calls
e.stopPropagation() before the event reaches React's synthetic
tree. The aside's onKeyDown never fires, so the thread stays open
while Esc just dismisses the dialog.
New test verifies the behavior: dialog mounted inside a parent
with its own onKeyDown spy; Esc fires onCancel but does NOT bubble
to the parent.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): user-reported UI issues — input CSS, header, flash-jump, dark-mode contrast
Six issues from manual smoke:
1. Message input was completely unstyled — the legacy .message-input
CSS targets the old <MessageInput> tree that was deleted in 3.8.
The new MessageInputForm uses .message-input-form + an inner
.message-input-row, neither of which had styles. Added a full
matching ruleset with consistent token usage so input + chip +
Send button all look like the rest of the app.
2. flash-jump animation never fired on quote-click because the
keyframe selector was .message.flash-jump but the new row class
is .message-row. Added a .message-row.flash-jump rule that
reuses the existing keyframes.
3. Hover-menu (Reply/Edit/Delete/Thread) was invisible in dark
mode — used hardcoded rgba fallbacks instead of theme tokens.
Switched to var(--bg-elevated) + var(--border-strong) +
--text-primary on the buttons + a subtle box-shadow so the
group reads against any background.
4. The 3-dot kebab visually collided with the action toolbar
because both were absolute-positioned at the top-right with
the same right offset. Pulled the toolbar left to right: 36px
so the kebab sits clear of it.
5. Kebab visibility was gated by .message:hover (legacy class) —
the kebab never appeared on the new .message-row tree.
Updated the selector to .message-row:hover + :focus-within so
keyboard users get it too. Added a hover background on the
kebab itself for affordance.
6. Reply-count badge used rgba fallback backgrounds that
disappeared on dark backgrounds; switched to --bg-elevated +
--border-strong + --text-primary for clarity in both themes.
Plus one related header change requested mid-fix: the ChatPage
room-header strip previously rendered the Members badge in the
middle and a Leave button on the right. The Leave button is gone
entirely (room exit lives elsewhere), and the Members badge is
right-justified via the existing flex spacer. Removed the now-
unused LeaveRoomButton import + isChannel constant.
403/403 tests still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): redesign message row — avatar + bubble + own-right
Three user-requested visual changes bundled into one structural
update of MessageRow.jsx + a fresh CSS section:
1. New row layout. Each message is now a flex row with an avatar
circle on the left (36px, initials placeholder until the avatar
service lands — colored background, sender's first letter) and
a column body on the right containing header (sender + time +
"(edited)") followed by an inline-block bubble with rounded
background that hugs the text.
2. Own messages flip right. Adding `message-row-own` reverses the
row direction (avatar on the right) and right-aligns the body
contents. The bubble switches to var(--accent-soft) — the same
token used for jump-flash and chat accents — giving own messages
the blue-ish tint requested.
3. Deleted messages are gone entirely. Previously rendered a
"[message deleted]" placeholder; the user wants the row removed
so the flow reads as if it never existed. MessageRow returns
null when message.deleted, and the test asserts no DOM output.
Additional polish that fell out:
- Hover-action toolbar mirrors to the left edge for own messages
(so the toolbar sits over the bubble's left top corner, near the
visible content rather than off-screen on the avatar side).
- Read-receipt kebab mirrors too — same idea, kept on the bubble
side, popover and tooltip flip with it.
- Inline edit input now has bubble-shaped chrome so the row's
rhythm doesn't break when editing.
- Reply-count badge + failed-row indicator now live inside the
body column so they align with the bubble.
403/403 tests still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): user-reported UI follow-ups — avatar, hover-on-bubble, kebab inside toolbar, deleted filter
Five visual changes from the user's smoke pass:
1. Own messages no longer render an avatar. The bubble alone (right-
aligned, blue tint) is enough to identify the sender; an avatar
on every own row is noise.
2. Others' avatars are flush to the left edge of the row. Row padding
asymmetric (8px left for non-own, 12px for own) so the avatar
doesn't sit under inset.
3. Hover trigger moved from `.message-row` to `.message-bubble-wrap`.
Hovering the avatar or header now does nothing visually. Only
hovering the bubble itself (or the actions over it) reveals the
action toolbar.
4. Kebab folded into the action toolbar. Previously the read-receipt
kebab was a separate floating element that drifted close to the
actions group; now it's the trailing button inside `.message-actions`
so the whole control surface reveals/dismisses as a single unit.
The popover (read-by-N-of-M) still anchors below the kebab and
flips left for own-messages so it doesn't escape the row.
5. Deleted messages filtered at MessageList. Previously MessageRow
short-circuited with `return null`, but the user reported still
seeing the avatar circle in some flow. Filter at the list level
so a deleted row is never even handed to MessageRow; the row
guard remains as defense-in-depth.
Plus dropped the row-wide hover background entirely — it conflicted
with bubble-scoped hover (the avatar area shouldn't tint as the
cursor passes through).
Also added a `Modal.jsx` base component (not yet wired) that will
host the edit + delete dialogs in the next commit. Touched it now
so the file lands without a dangling untracked status.
403/403 tests still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): main-feed optimistic dispatch + QuotedBlock id fallback
User reported: when replying with a quote in the main feed, the reply
shows but the quoted message above it does NOT — for both own and
others' messages.
Root causes:
1. Main-feed sends had no optimistic dispatch. The user's reply only
appeared after the broadcast loop returned, and if the broadcast
omitted/lost the resolved quotedParentMessage snapshot the bubble
above the reply showed nothing.
2. QuotedBlock's click-to-jump used snapshot.id, which exists only
on optimistic / client-staged snapshots. Server-side
cassandra.QuotedParentMessage uses messageId — clicks were
silently no-ops on server-delivered quotes.
Changes:
- New MESSAGE_SENT_LOCAL action in roomEventsReducer. Mirrors the
thread reducer's REPLY_SENT_LOCAL: appendBounded with id-dedupe,
carries `_local: true`. When a real broadcast for the same id
arrives later, MESSAGE_RECEIVED's "skip duplicate" path is a no-op
and the optimistic stays in place.
- RoomMessageInput dispatches the new action right before the
publish, with an optimistic message shaped like a server message
(sender object + ISO createdAt). The quotedParentMessage carries
the server-shaped snapshot fields (messageId, sender, msg) so
QuotedBlock's fallback chain renders correctly with no special-
casing.
- QuotedBlock now reads `snapshot.messageId || snapshot.id` for the
jump target, so click-to-jump works for both server and client
snapshots. Bails out silently if neither is set.
- New reducer tests (3) + RoomMessageInput tests (2) cover the
optimistic path end-to-end.
- ChatPage.test.jsx mocks updated to expose useRoomDispatch — the
hoisted mock used by the E2E test now mirrors the real export.
408/408 tests passing.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* feat(chat-frontend): edit moves from inline to modal; reusable Modal + TextInputDialog
User request: both edit and delete should open a centered modal with
a greyed-out backdrop, and the two should share a reusable dialog
primitive.
Pieces:
- New Modal component (chat-frontend/src/components/messages/Modal.jsx)
is the reusable base: greyed overlay + centered card, backdrop-
click closes, Esc closes via capture-phase + stopPropagation (so
it doesn't trip ThreadRightBar's own Esc handler). role="dialog"
+ aria-modal + caller-supplied aria-labelledby for screen readers.
- New TextInputDialog (TextInputDialog.jsx) — small reusable modal
for editing one line of text. Auto-focuses + selects, Enter saves,
Save disabled until non-empty AND different from initial. Built
on Modal.
- DeleteConfirmDialog rewritten on top of Modal — same call signature
(onConfirm/onCancel/pending) but now uses the proper .dialog-overlay
class so the backdrop actually greys out, matching CreateRoomDialog
/ ManageMembersDialog conventions.
- Inline edit mode in MessageRow REMOVED. The bubble no longer swaps
for an <input> when editing; the row only ever renders the bubble.
Edit state moves up to RoomMessageArea / ThreadMessageArea, which
track `editingMessage` (the full object, not just id) and render
a TextInputDialog when set. On save: same publish + dispatch path
as before, just driven from the dialog's onSave callback.
- MessageList dropped the editing-related forwarded props
(editingMessageId / onEditSubmit / onEditCancel) — those no longer
flow through the row.
- Tests updated end-to-end: MessageRow tests drop the inline-edit
cases (3 deleted). RoomMessageArea + ThreadMessageArea now drive
the new flow by clicking the dialog's Save / Cancel buttons,
asserting the dialog mounts with the right pre-fill, and confirms
it dismisses on save / cancel. 406/406 tests passing.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): hover-stay toolbar, inline quote inside bubble, auto-focus on reply
User reports:
- Replied-quote doesn't render anymore (regression from UI redo)
- Thread button sometimes works on own messages, never on others'
- Reply button clicks: have to manually click input afterward
- Horizontal scrollbar from hover menu
Root cause for the disappearing-menu bugs (Thread + Reply): the
hover toolbar was floating ABOVE the bubble at `top: -14px` but
the bubble-wrap's hover area was ONLY the bubble's rectangle. Mouse
moving up to click an action left the hover zone, opacity went to
0, pointer-events: none, and the click was eaten by whatever sat
behind it. On own messages it worked sometimes because the toolbar
extended into the body area where hover lingered briefly.
Fixes:
1. Hover-stay. Toolbar still floats above the bubble but uses
`transition: opacity 80ms ease 250ms` for HIDE (immediate show,
delayed hide). `.message-actions:hover` and `:focus-within`
also keep it visible+clickable, so the moment the cursor
crosses from bubble onto toolbar, hover is preserved.
2. Smart anchor for own messages. Toolbar anchored at `left: -2px`
for own (bubble lives at the right edge of the row, so the
toolbar extends LEFTWARD into open space) and `right: -2px`
for others'. Combined with overflow-x: hidden on .message-list,
this kills the horizontal scrollbar.
3. Quote rendered INSIDE the bubble. Moved <QuotedBlock> from a
sibling of .message-bubble-wrap to a child of .message-bubble.
The bubble auto-sizes to include the quote card; the quote
reads as "this is what I'm replying to" with a left-bar +
faded bg. New `.message-bubble-content` for the content line.
This fixes the "quote doesn't render" regression — block-level
QuotedBlock inside the inline-block bubble flows correctly,
whereas it was getting laid out weirdly as a sibling.
4. Auto-focus on staged-quote. RoomMessageInput and
ThreadMessageInput now hold a ref to their <input> and focus
it whenever quotedTarget?.id changes — so clicking Reply jumps
the cursor straight into the input without an extra click.
5. overflow-x: hidden on .message-list to clip any residual
toolbar overflow on very narrow rows.
406/406 tests still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* fix(chat-frontend): JS-driven hover (clickable Thread/Reply), smart anchor, visible quote
User reports persist:
- Thread button shows but click doesn't open the right panel
- Reply: quoted message doesn't render above own reply
- Menu on own messages overflows the right edge of the screen
Root causes:
1. CSS-only :hover + transition-delay is fragile. The instant the
cursor leaves the .message-bubble-wrap's geometric box,
pointer-events flips to 'none' (transitions don't apply to
pointer-events), and the Thread / Reply click is eaten by
whatever's underneath. transition-delay on opacity only delays
the VISUAL fade; it doesn't keep the element clickable.
2. Smart positioning was backwards. `.message-row-own .message-actions
{ left: -2px }` anchored the menu's LEFT edge 2px past the
bubble's left, making it extend RIGHTWARD by the toolbar's width
— straight off the viewport for own messages (bubble already at
the right edge).
3. Quote-not-rendering was a contrast bug: `--bg-quoted` and the
inline rgba fallback (3% black) are imperceptible in dark mode.
The quote IS rendered, just invisible.
Fixes:
A. New `useHoverWithDelay(delayMs)` hook in src/lib/. Returns
`{ hovered, handlers }` and exposes onMouseEnter / onMouseLeave
spreaders. Attach the SAME handlers to both the bubble-wrap and
the floating action-host so the menu stays mounted while the
cursor crosses between them. 200ms delay before hide.
B. MessageRow now uses the hook. The action toolbar is conditionally
rendered (`{hovered && <div className="message-actions-host">…}`),
so there's no opacity / pointer-events trickery. It's either in
the DOM (and clickable) or not at all.
C. Positioning moved to `.message-actions-host` so the host owns the
geometric box (size + position). Smart anchor flipped:
- Others: `left: 0` (extends rightward into open space)
- Own: `right: 0` (extends leftward into open space)
Both keep the menu inside the row's bounds — no viewport overflow.
D. Quote rendered as a SIBLING above the bubble (inside the wrap),
styled with theme tokens (`var(--bg-hover)`, `var(--accent)` left
border) so it actually shows in both light and dark themes.
E. Tests updated: any test that asserts on the kebab or the action
buttons now fires a `mouseEnter` on `.message-bubble-wrap` first
to mount the menu.
406/406 tests still green.
https://claude.ai/code/session_01LL1Y8mp4gjjGL4PoUUZgfL
* test(chat-frontend): integration test for Thread click → panel mount
Adds a high-fidelity integration test that mounts MainApp with the REAL
NatsContext/RoomEventsContext/ThreadEventsContext providers (only the
NATS transport is faked at the bottom layer). Simulates a Thread button
click and asserts the ThreadRightBar appears in the DOM with the real
ThreadMessageArea and ThreadMessageInput mounted inside it.
The existing MainApp.test.jsx mocks ThreadEventsContext directly, which
trivially passes the activeParent → ThreadRightBar mount assertion. This
test closes that gap — it exercises the full chain so a regression in
the click handler, the openThread callback, the reducer, or the panel
mount would surface here.
Confirms: the click → panel chain works end-to-end in jsdom.
* fix(chat-frontend): unambiguous text labels on hover toolbar
The icon-only toolbar (💬 Thread, ↩ Quote, ✎ Edit, 🗑 Delete) was too easy
to confuse — both Thread and Quote use chat-bubble glyphs and look
identical to most users. Replaced with text labels: Thread / Quote /
Edit / Delete, plus matching `title=` tooltips. Buttons are sized down
slightly (0.8em, padding 2px 8px) so the toolbar still fits compactly
over the bubble.
* fix(chat-frontend): ThreadMessageArea passes room to MessageList
The kebab (MessageActionMenu) returns null when room?.id is falsy. In
the main feed, RoomMessageArea passes room → MessageList → MessageRow →
MessageActions → MessageActionMenu and the kebab renders. In the
thread panel, ThreadMessageArea never passed room down, so the kebab
silently disappeared for every message inside a thread.
Resolve the room from useRoomSummaries() using activeParent.roomId,
defaulting to null if the summary isn't loaded yet.
* fix(chat-frontend): normalize historical messages to broadcast shape
ROOT CAUSE of "Thread doesn't open on historical messages after refresh":
The chat API exposes two different shapes for the same message:
- Broadcast events use pkg/model.Message → fields: id, content,
userAccount, createdAt.
- History RPCs return pkg/model/cassandra.Message → fields:
messageId, msg, sender.account, createdAt.
Everything in the frontend (reducers, MessageRow, MessageList, click
handlers, dedup, optimistic dispatch) reads `msg.id` and `msg.content`,
i.e. the broadcast shape. Optimistic sends and live broadcasts both
arrive in that shape, so they work. But after a page refresh — when
every visible message comes from history — `msg.id` is undefined
everywhere. Symptoms:
- ChatPage.handleThread calls openThread({ messageId: undefined })
→ activeParent is set with a garbage id, msg.thread RPC fails
silently, and the user perceives "Thread button does nothing".
- mergeById and MESSAGE_RECEIVED dedup collapse undefined === undefined
so historical messages can confuse the live merge.
- MessageActionMenu's room-id check would also miss.
Fix: introduce normalizeHistoricalMessage at the load boundary
(loadHistory, jumpToMessage via msgSurrounding, ThreadEvents openThread
HISTORY_LOADED). The normaliser maps cassandra.Message → broadcast
shape (messageId → id, msg → content) and preserves every other field
(sender, createdAt, quotedParentMessage, tcount, editedAt, deleted).
The reducer + UI stay broadcast-shape-only.
8 new tests cover the normalizer.
* fix(chat-frontend): five UX issues — popover, toolbar, quote-hover, jump persistence, thread-quote sender
1) Kebab popover overflow on own messages
The own-side override anchored the popover to `left: 0` of the kebab,
which made it open RIGHTWARD past the bubble's right edge and off the
viewport. The default `right: 0` is correct for both own and others
(kebab only renders on own anyway), so the override was deleted.
2) Empty toolbar on the thread panel's parent message
`showThread = context !== 'thread-parent'` and the same gate on Quote
left the toolbar empty for OTHERS' thread-parent messages (no Edit /
Delete either, since not own). Reworked:
- Thread → only `context === 'main'` (already in a thread otherwise)
- Quote → always (any visible message can be quoted in a reply)
- Edit / Delete / Kebab → own-only
Added a guard so MessageActions returns null if the toolbar would be
completely empty (no buttons + no kebab).
3) Quoted block triggered the hover menu
QuotedBlock was rendered INSIDE .message-bubble-wrap, which owned the
hover handlers — so hovering the quote opened the action toolbar.
Lifted QuotedBlock to be a sibling of bubble-wrap under
.message-row-body and updated the CSS selector + alignment so it still
sits flush above the bubble (own-side aligns flex-end).
4) Jump-to-message stuck across room switches and silent on second click
`focusMessageId` lived in roomState forever after a jump. Switching
away and back replayed the flash; clicking the same quote twice was a
no-op (the focus effect's deps didn't change). Added a FOCUS_CLEARED
reducer action and an `onFocusConsumed` callback on MessageList that
RoomMessageArea wires to dispatch it after the scroll/flash kicks off.
5) Thread-quote sender rendered "unknown"
ThreadMessageInput passed only `quotedParentMessageId` to sendReply,
so the optimistic `quotedParentMessage` had `senderName: undefined` /
`content: undefined` and QuotedBlock fell through to "Unknown" until
the server broadcast arrived. Now passes `quotedSnapshot: { senderName,
content }` alongside the id, and ThreadEventsContext builds the
optimistic snapshot in the server's cassandra shape (messageId / sender
/ msg) so it renders identically to the eventual broadcast / history
reload. Updated retryReply's id read for the same field rename.
* style(chat-frontend): comprehensive UI polish — design tokens, button system, dark mode
DESIGN TOKENS (tokens.css)
Reorganised into two layers:
- Primitives: spacing scale (--space-2xs..2xl), radius scale (sm/md/lg/xl/
pill/circle), type scale (--text-xs..xl), font weights, line heights,
motion timing + easing.
- Semantic roles: surfaces, text, borders, accent + accent-soft-hover,
status (incl. --status-error-hover), bubbles (--bubble-own /
--bubble-other), border-focus, focus-ring composite, shadow-sm/md/dialog.
Added the previously-missing --text-error / --focus-ring / --status-error-hover
tokens so any fallback literal colors are gone.
DESIGN PRIMITIVES (index.css top)
Added a global focus-ring rule that fires on :focus-visible for every
interactive element (button, input, textarea, [tabindex]) — keyboard users
get a consistent accent ring, mouse clicks don't.
Added unified .btn family (.btn, .btn-primary, .btn-ghost, .btn-danger,
.btn-icon) for any future buttons; existing one-off styles now share the
same vocabulary.
Added themed thin scrollbars for every scroll container so dark mode
doesn't expose OS chrome.
POLISHED SURFACES
- Login: padded outer container, larger card, hover/focus states on
input, consistent spacing scale.
- App header + sidebar: bottom border, tightened title typography,
sidebar gets its own right border and bg-sidebar background.
- Room list: items have hover transitions, selected uses bg-selected
consistently in light + dark.
- Chat room header: 56px min-height, consistent letter-spacing.
- Message row: cleaner typography (--text-sm sender, --text-xs time),
avatar uses accent color for initial, own-message bubble uses
--bubble-own (more saturated tint in both themes), quote sits flush
above bubble with accent left/right border depending on side, quote
has its own hover state.
- Hover toolbar: tighter padding, --shadow-md elevation, delete button
hover turns red, smooth transitions.
- Kebab popover + read-receipts: bg-surface (not elevated), wider
min-width, consistent type sizes.
- Reply-count badge: pill-shape, accent-soft background — clear "open
thread" affordance.
- Failed-row UI: error-bordered chip rather than inline red text.
- Inputs (login, message-input-form, in-room-search, dialog): consistent
bg-input → bg-input-hover → bg-surface progression with 3px focus halo.
- Dialog: subtle border, larger shadow, backdrop blur, themed checkboxes
(accent-color: var(--accent)).
- Thread panel: matches main feed header (56px), themed close button,
accent-bordered quote chip in the input.
- Members badge / search bar / in-room search: pill / pill-input
treatment, consistent 3px focus halo.
- Member picker: full styling pass (was nearly unstyled) — chips use
accent-soft pill, focus-within border on the input wrap, themed
dropdown.
- Manage Members tabs: muted → primary on hover, accent underline on
active, breaks out of dialog padding so the underline runs edge-to-edge.
DARK MODE
All component colors now flow from semantic tokens, so light/dark theme
parity is automatic. No literal hex / rgba colors remain anywhere in
index.css (verified). The only previously-leaky token (--text-error) is
now defined in both themes.
* style(chat-frontend): visible search-bar border + Add-members spacing
- .search-bar now uses a solid --border-strong border in the rest state
(was transparent, which let the input blend into the header). Hover
promotes the border to --text-muted so the interactive affordance
reads clearly in both themes.
- Add-members modal had two spacing issues:
1) the share-history checkbox was flush against the MemberPicker chip
input, and
2) the Close button sat too far below the inner Add-button row.
The <form> inside .manage-members-dialog is now a flex column with
--space-md gap, so MemberPicker / checkbox / error / Add-row space
evenly. The .manage-members-footer margin-top is dropped — the
.dialog flex parent already provides the gap, and stacking another
--space-md on top was what made Close drop too low.
* fix(chat-frontend): Add-members layout — checkbox inline, buttons in one row
Two issues in the Add-members tab of ManageMembersDialog:
1. The share-history checkbox sat on its own line above the label text.
The global ".dialog input { display: block; width: 100% }" rule was
also stretching <input type="checkbox"> to full width, forcing the
text node to wrap below. Scoped the block rule to
":not([type='checkbox']):not([type='radio'])" so checkboxes keep
their natural inline sizing.
2. Close and Add stacked vertically because Close lived in
ManageMembersDialog's outer footer while Add lived in
AddMembersForm's own .dialog-actions. Moved Close into the Add tab's
action row (passed via onClose prop). The outer footer now renders
only on the Members tab, where there's no inner Add row. The shared
.dialog-actions row uses "margin-right: auto" on .dialog-cancel to
push the primary action right while keeping cancel left — Close on
the left, Add on the right, in one line.
* refactor(chat-frontend): nest components into folder-per-component structure
Each component now lives in its own folder. The component file keeps
its descriptive name (e.g. MainApp/MainApp.jsx) and the folder has an
index.jsx that re-exports the default — so existing import paths like
`import MainApp from './MainApp'` continue to resolve through the
folder/index.jsx pattern.
Component tree mirrors the runtime tree:
components/
MainApp/
MainApp.jsx
index.jsx
AppHeader/
SearchBar/
ThemeToggle/
Sidebar/
RoomList/
CreateRoomDialog/
ChatPage/ (moved from src/pages/)
RoomMessageArea/
RoomMessageInput/
InRoomSearch/
LeaveRoomButton/
RoomMembersBadge/
ManageMembersDialog/
AddMembersForm/
MemberRoster/
MemberPicker/
ThreadRightBar/
ThreadMessageArea/
ThreadMessageInput/
SearchResultsPane/ (moved from src/pages/)
shared/
MessageList/
MessageRow/
MessageActions/
MessageActionMenu/
MessageInputForm/
QuotedBlock/
Modal/
DeleteConfirmDialog/
TextInputDialog/
components/shared/ holds anything used by more than one branch of the
tree (MessageList is used by both RoomMessageArea and ThreadMessageArea;
MessageInputForm by both message inputs; Modal by DeleteConfirmDialog +
TextInputDialog + CreateRoomDialog; QuotedBlock by MessageRow +
MessageInputForm).
src/pages/ retains only LoginPage and OidcCallback — they're pre-auth,
not children of MainApp.
All relative imports + vi.mock paths regenerated to match the new
depths. 416/416 tests pass, production build clean.
CSS still lives in src/styles/index.css for this commit; the per-component
style split lands in the next commit.
* refactor(chat-frontend): split CSS per component, trim shared index.css
Each component folder now owns its own style.css with the selectors
scoped to that component. The component's main file imports it via
`import './style.css'`, so Vite bundles the rules alongside the
component without any further wiring.
What stayed in src/styles/index.css (truly app-wide):
- CSS reset (*, html, body)
- typography defaults on body
- global :focus-visible ring on interactive elements
- .btn family (.btn, .btn-primary, .btn-ghost, .btn-danger, .btn-icon)
- Themed thin scrollbars (.scrollbar-thin + class enumeration)
- Dialog primitives (.dialog-overlay, .dialog, .dialog-actions,
.dialog-error/success/checkbox/cancel, base input/select/textarea
inside .dialog) — shared across CreateRoomDialog, ManageMembers,
DeleteConfirmDialog, TextInputDialog, …
- flash-jump @keyframes (the .message-row.flash-jump rule that
consumes it…
7 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
End-to-end rebuild of the chat-frontend's create-room and manage-members flows to speak the room-service async-job protocol correctly, plus a member roster with inline actions, inline org expansion, DM display-name fallback, and encryption-resilient broadcast rendering. 14 commits; no backend changes.
Design spec:
docs/superpowers/specs/2026-05-13-chat-frontend-room-operations-design.md.What changed (by layer)
lib/asyncJob.js—requestWithAsyncResult(nc, account, subject, payload, opts)bridges room-service's two-phase reply contract (sync ack +AsyncJobResultonchat.user.{a}.response.{rid}). Subscribe-before-publish,X-Request-IDheader, tagged errors viaerr.kind,treatAsSuccesspredicate (used for DM-exists dedup).NatsContext—requestWithAsyncResulton the context value (injectsnc+user.account),useMemo'd value, JSDoc on every callback.MemberPicker— single chip-input for users / orgs / channels with channel typeahead viasearch.rooms. Comma-separated parsing ("alice, bob, charlie"+ Enter → 3 chips).forwardRef+useImperativeHandleexposingflushAndGetEntries()so submit handlers can auto-commit typed-but-not-Entered text.CreateRoomDialog— rewired toroomCreate(account, siteId)with{name, users, orgs, channels}payload (old subject didn't match any handler). DM dedup viaisDMExistsReply. Parks open withWaiting for server confirmation…untilsubscription.updatelands; 3s timeout surfaces an in-dialog error (does not auto-select to avoid bouncing the user back to the empty state).MemberRoster— single ordered list (orgs first, individuals second), EngName + ChineseName for individuals, owner-gated Promote/Demote/Remove, self-row Leave with confirm, inline org expansion viachat.user.{a}.request.orgs.{orgId}.memberswith chevron + cached children + loading/error states. Generation-counter guards onfetchMembers+ per-orgtoggleOrgto drop stale async writes.ManageMembersDialog— 4 typed-text tabs collapsed to 2 (Members / Add). RemovedLeaveRoomButton,RemoveMemberForm,RoleUpdateForm,RemoveOrgForm,RemoveByIdRow. Leave reachable only via the roster's self-row.AddMembersForm— rewritten to useMemberPicker; sendsChannelRef[]for channels (was sending strings — silent no-op bug).RoomMembersBadge/MessageArea—N membersbadge insideMessageArea's header (replaces the old static span). Click opensManageMembersDialog.ChatPagebumps amembersRefreshKeyon dialog close.roomDisplayName(room)helper: preferroom.name, thenroom.subscriptionName, then(DM)placeholder.RoomEventsContextmergesevt.subscription.nameonto rooms before dispatchingROOM_ADDED.MessageInputplaceholder uses the helper too (Message @ bobfor DMs instead ofMessage #bob-dm).[encrypted message]placeholder when onlyevt.encryptedMessageis present (previously silently dropped).console.debuginstrumentation on every step of the receive path for tracing.scripts/asyncJob.smoke.mjs(wire-level vs vanilla NATS) andscripts/liveStack.smoke.mjs(e2e vs auth-service + room-service + room-worker).Test plan
cd chat-frontend && npx vitest run→ 268 pass / 0 fail across 26 filescd chat-frontend && npm run build→ ✓ built cleanWaiting…untilsubscription.updatelands; expand the org row to see its members(DM)fallback ifsubscription.namehasn't landed)BROADCAST_ENCRYPTION_ENABLED=trueon broadcast-worker, send a channel message and verify the[encrypted message]placeholder appears🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
UI/UX Improvements
Bug Fixes
Tests
Removals
Documentation