Skip to content

feat(audio): persistent audio player across room navigation#41120

Open
ggazzo wants to merge 21 commits into
developfrom
feat/persistent-audio-player
Open

feat(audio): persistent audio player across room navigation#41120
ggazzo wants to merge 21 commits into
developfrom
feat/persistent-audio-player

Conversation

@ggazzo

@ggazzo ggazzo commented Jun 30, 2026

Copy link
Copy Markdown
Member

Proposed changes

Playing an audio message attachment used to stop the moment you switched or closed the conversation, because the <audio> element lived inside the message component and was destroyed when the message list unmounted.

This PR makes audio playback persistent:

  • A single shared <audio> element is hoisted above the room layout into a new MediaPlayerProvider. Because it lives above the room, it survives navigation and message-list unmounting, so playback continues uninterrupted.
  • A "Now playing" card pinned to the top of the sidebar reflects the shared player — play/pause, seek, playback speed (1x / 1.5x / 2x), and a shortcut back to the originating conversation.
  • In-message audio controls now drive the same shared element, so starting playback in a message and the sidebar card stay in sync.
  • Signed-URL expiry recovery (previously in useReloadOnError) is ported into the provider so the persistent element keeps recovering expired media URLs mid-playback.

Architecture

Piece Responsibility
providers/MediaPlayerProvider Owns the single hidden <audio> element + playback state (track, playing, currentTime, duration, playbackRate) and actions (play, toggle, seek, cyclePlaybackRate, close, isActive).
components/AudioPlayer/AudioPlayerControls Reusable play / seek / time / speed controls (with a compact mode for the sidebar).
sidebar/sections/NowPlayingSection The sidebar "Now playing" card, re-attaching to the shared player.
AudioAttachment Drives the shared player instead of a self-contained widget.

Message metadata (rid, mid, sender) is threaded through the attachment chain so the card can show the sender and jump back to the conversation.

How to test

  1. Open a conversation containing an audio attachment and start playback.
  2. Switch to another room or close the conversation — playback continues.
  3. The "Now playing" card appears at the top of the sidebar; verify play/pause, seek, speed cycling, and the jump-back shortcut.

Further comments

UI follows the sidebar card placement. The provider is placement-agnostic, so a floating / footer / topbar dock could reuse the same context later.

Review in cubic

image

Follow-up (TODO)

The in-message/card controls (apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx) are a temporary local copy of a controlled component being upstreamed to Fuselage: RocketChat/fuselage#2052. Once that ships in @rocket.chat/fuselage, delete the local copy (and formatPlaybackTime) and import AudioPlayerControls from @rocket.chat/fuselage instead.

Task: ARCH-2202

Summary by CodeRabbit

  • New Features
    • Added persistent audio playback for audio attachments that continues across room navigation (and keeps playing when switching/closing conversations).
    • Added a pinned Now playing card in the sidebar with play/pause, seek, playback speed (1x/1.5x/2x), and a jump back to the originating conversation.
  • Bug Fixes
    • Improved playback resilience by recovering automatically when signed media links fail or expire.
  • Documentation
    • Added new i18n labels for “Now playing”, “Playback speed”, and “Seek”.

Audio message playback no longer stops when you switch or close the
conversation. A single shared audio element is hoisted above the room
layout into a new MediaPlayerProvider, so it survives message-list
unmounting and keeps playing while the UI re-attaches to it.

A "Now playing" card pinned to the top of the sidebar reflects the
shared player: play/pause, seek, playback speed (1x/1.5x/2x), and a
shortcut back to the originating conversation. In-message audio
controls drive the same shared element.
@dionisio-bot

dionisio-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Looks like this PR is not ready to merge, because of the following issues:

  • This PR is missing the 'stat: QA assured' label

Please fix the issues and try again

If you have any trouble, please check the PR guidelines

@changeset-bot

changeset-bot Bot commented Jun 30, 2026

Copy link
Copy Markdown

🦋 Changeset detected

Latest commit: bc0c16a

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages
Name Type
@rocket.chat/meteor Minor
@rocket.chat/i18n Minor
@rocket.chat/core-typings Minor
@rocket.chat/rest-typings Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@coderabbitai

coderabbitai Bot commented Jun 30, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7c9b8b46-8f69-49c7-ba2d-f85150cc6103

📥 Commits

Reviewing files that changed from the base of the PR and between 6f13287 and bc0c16a.

📒 Files selected for processing (2)
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
📜 Recent review details
⏰ Context from checks skipped due to timeout. (4)
  • GitHub Check: 📦 Build Packages
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: CodeQL-Build
  • GitHub Check: CodeQL-Build

Walkthrough

This PR adds a persistent shared audio player, threads audio attachment source metadata through message rendering, and surfaces the active track in a new sidebar “Now Playing” card with playback controls and navigation back to the source conversation.

Changes

Persistent Audio Player Feature

Layer / File(s) Summary
Media player context and playback controls
apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts, apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx, apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
Defines PersistentAudioTrack, MediaPlayerContextValue, useMediaPlayer(), the reusable AudioPlayerControls UI, and the formatPlaybackTime helper.
MediaPlayerProvider implementation and app wiring
apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx, apps/meteor/client/providers/MediaPlayerProvider/index.ts, apps/meteor/client/providers/MeteorProvider.tsx
Implements a shared hidden <audio> element with signed-URL recovery, playback controls, barrel exports, and mounts the provider in MeteorProvider.
Attachment components pass audio source metadata
apps/meteor/client/components/message/content/Attachments.tsx, .../attachments/AttachmentsItem.tsx, .../attachments/FileAttachment.tsx, .../attachments/file/AudioAttachment.tsx, .../variants/room/RoomMessageContent.tsx, .../variants/thread/ThreadMessageContent.tsx
Adds optional source (AudioAttachmentSource) propagation from message content to AudioAttachment, which now builds a persistent track and uses useMediaPlayer/AudioPlayerControls.
Sidebar Now Playing card
apps/meteor/client/sidebar/sections/SidebarCard.tsx, apps/meteor/client/sidebar/sections/NowPlayingSection.tsx, apps/meteor/client/sidebar/Sidebar.tsx, apps/meteor/client/views/navigation/sidebar/Sidebar.tsx
Adds a reusable sidebar card and a NowPlayingSection showing the active track with controls, avatar, close button, and navigation back to the originating conversation; mounted in both sidebar layouts.
Localization strings and changeset
packages/i18n/src/locales/en.i18n.json, .changeset/persistent-audio-player.md
Adds Now_playing, Playback_speed, and Seek strings and documents the feature with minor bumps for @rocket.chat/meteor and @rocket.chat/i18n.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant User
  participant AudioAttachment
  participant MediaPlayerProvider
  participant NowPlayingSection
  participant AudioElement

  User->>AudioAttachment: click play on message attachment
  AudioAttachment->>MediaPlayerProvider: play(track)
  MediaPlayerProvider->>AudioElement: set src, load, play
  MediaPlayerProvider-->>NowPlayingSection: context updates (track, playing)
  User->>NowPlayingSection: navigate to another room
  NowPlayingSection->>MediaPlayerProvider: playback persists
  User->>NowPlayingSection: click header
  NowPlayingSection->>User: navigate to originating room
  User->>NowPlayingSection: click close
  NowPlayingSection->>MediaPlayerProvider: close()
  MediaPlayerProvider->>AudioElement: pause, clear src, reset state
Loading

Suggested labels: type: feature

Suggested reviewers: tassoevan

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: persistent audio playback across room navigation.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • ARCH-2202: Request failed with status code 401

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@codecov

codecov Bot commented Jun 30, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 35.71429% with 288 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.94%. Comparing base (b6e0d3b) to head (bc0c16a).
⚠️ Report is 4 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop   #41120      +/-   ##
===========================================
+ Coverage    69.14%   69.94%   +0.80%     
===========================================
  Files         3433     3374      -59     
  Lines       132323   130860    -1463     
  Branches     23091    22705     -386     
===========================================
+ Hits         91489    91528      +39     
+ Misses       37472    36015    -1457     
+ Partials      3362     3317      -45     
Flag Coverage Δ
unit 69.85% <35.71%> (-0.15%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

ggazzo added 19 commits June 30, 2026 15:39
The in-message audio player is restored to the native fuselage AudioPlayer
(unchanged UI/behavior). Persistence now works as a hand-off: a usePersistentAudio
hook observes the message's audio element and, when the message unmounts while
playing, hands the track to the shared detached element in MediaPlayerProvider so
playback continues in the sidebar card. Re-entering the room hands the track back
and resumes in the message element.
Replace the two-element hand-off with one shared <audio> element owned by
MediaPlayerProvider and never recreated. Both the in-message controls and the
sidebar card drive that same element, so switching or closing the room only
swaps which UI renders — playback continues with no reload, seek, or gap.

Removes usePersistentAudio (hand-off no longer needed).
@ggazzo ggazzo added this to the 8.7.0 milestone Jul 1, 2026
@ggazzo ggazzo marked this pull request as ready for review July 1, 2026 20:11
@ggazzo ggazzo requested a review from a team as a code owner July 1, 2026 20:11
@ggazzo

ggazzo commented Jul 1, 2026

Copy link
Copy Markdown
Member Author

/jira ARCH-2167

@coderabbitai coderabbitai Bot added the type: feature Pull requests that introduces new feature label Jul 1, 2026

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🧹 Nitpick comments (4)
apps/meteor/client/sidebar/sections/NowPlayingSection.tsx (1)

13-18: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove implementation comment per coding guidelines.

As per coding guidelines, **/*.{ts,tsx,js} files should "Avoid code comments in the implementation."

🤖 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 `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx` around lines 13 -
18, Remove the implementation comment from NowPlayingSection so the TSX file
follows the no-comments guideline; delete the block above the component in
NowPlayingSection and keep the code unchanged.

Source: Coding guidelines

apps/meteor/client/sidebar/sections/SidebarCard.tsx (1)

6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Remove implementation comment per coding guidelines.

As per coding guidelines, **/*.{ts,tsx,js} files should "Avoid code comments in the implementation." This JSDoc block should be removed or the description embedded in the PR/documentation instead.

🤖 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 `@apps/meteor/client/sidebar/sections/SidebarCard.tsx` around lines 6 - 9,
Remove the implementation JSDoc comment from SidebarCard since the coding
guidelines discourage code comments in tsx implementation files. Update the
SidebarCard component by deleting the descriptive block above the reusable
elevated surface logic, and keep any needed explanation in external
documentation or the PR description instead.

Source: Coding guidelines

apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx (1)

6-9: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Move this upstream-tracking TODO out of the component file.

The repo guideline for TS/TSX files asks us to avoid implementation comments. Please track the planned Fuselage swap in the PR/issues instead of keeping the TODO block in shipped code. As per coding guidelines, "**/*.{ts,tsx,js}: ... Avoid code comments in the implementation`."

🤖 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 `@apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx` around
lines 6 - 9, The upstream-tracking TODO in AudioPlayerControls is an
implementation comment that should not live in shipped TSX code. Remove the TODO
block from AudioPlayerControls and track the Fuselage swap in the PR or issue
instead, keeping the component file focused on implementation only.

Source: Coding guidelines

apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx (1)

13-19: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win

Drop the docblock from this implementation file.

The repo guideline explicitly avoids code comments in implementation, and AudioAttachmentSource is already self-descriptive. As per coding guidelines, "Avoid code comments in the implementation."

Suggested cleanup
-/** Extra context about the message that owns this audio, used by the shared player. */
 export type AudioAttachmentSource = {
 	rid?: string;
 	mid?: string;
 	username?: string;
 	name?: string;
 };
🤖 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
`@apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx`
around lines 13 - 19, Remove the docblock above AudioAttachmentSource in
AudioAttachment.tsx, since this is an implementation file and the type is
already self-descriptive. Keep the AudioAttachmentSource export unchanged and
only delete the comment so the component file follows the repo rule against code
comments in implementation.

Source: Coding guidelines

🤖 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 `@apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx`:
- Around line 64-117: The recovery flow in handleMediaURLRecovery should verify
the active track still matches after awaiting getRedirectURLInfo() and avoid
leaving isRecoveringRef.current stuck true on early returns. Move the in-flight
guard so it only stays set for the actual recovery work, re-check
trackRef.current/current identity after the await before assigning node.src, and
always clear isRecoveringRef.current in a finally path when the attempt is
abandoned or fails.

In `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx`:
- Around line 43-51: The clickable Box in NowPlayingSection only supports mouse
interaction, so keyboard users can’t trigger openConversation. Update the Box
that currently uses onClick/openConversation to behave like an accessible button
by adding an appropriate role, tabIndex, and key handling (for Enter/Space), and
keep the pointer cursor conditional on track.rid. Use the NowPlayingSection and
openConversation identifiers to locate the interactive card area.

---

Nitpick comments:
In `@apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx`:
- Around line 6-9: The upstream-tracking TODO in AudioPlayerControls is an
implementation comment that should not live in shipped TSX code. Remove the TODO
block from AudioPlayerControls and track the Fuselage swap in the PR or issue
instead, keeping the component file focused on implementation only.

In
`@apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx`:
- Around line 13-19: Remove the docblock above AudioAttachmentSource in
AudioAttachment.tsx, since this is an implementation file and the type is
already self-descriptive. Keep the AudioAttachmentSource export unchanged and
only delete the comment so the component file follows the repo rule against code
comments in implementation.

In `@apps/meteor/client/sidebar/sections/NowPlayingSection.tsx`:
- Around line 13-18: Remove the implementation comment from NowPlayingSection so
the TSX file follows the no-comments guideline; delete the block above the
component in NowPlayingSection and keep the code unchanged.

In `@apps/meteor/client/sidebar/sections/SidebarCard.tsx`:
- Around line 6-9: Remove the implementation JSDoc comment from SidebarCard
since the coding guidelines discourage code comments in tsx implementation
files. Update the SidebarCard component by deleting the descriptive block above
the reusable elevated surface logic, and keep any needed explanation in external
documentation or the PR description instead.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 41407861-799c-4a26-8489-3af75a90f723

📥 Commits

Reviewing files that changed from the base of the PR and between b6e0d3b and 6f13287.

📒 Files selected for processing (18)
  • .changeset/persistent-audio-player.md
  • apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx
  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/components/message/content/Attachments.tsx
  • apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx
  • apps/meteor/client/components/message/content/attachments/FileAttachment.tsx
  • apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
  • apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
  • apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MeteorProvider.tsx
  • apps/meteor/client/sidebar/Sidebar.tsx
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
  • apps/meteor/client/sidebar/sections/SidebarCard.tsx
  • apps/meteor/client/views/navigation/sidebar/Sidebar.tsx
  • packages/i18n/src/locales/en.i18n.json
📜 Review details
⏰ Context from checks skipped due to timeout. (2)
  • GitHub Check: cubic · AI code reviewer
  • GitHub Check: Hacktron Security Check
⚠️ CI failures not shown inline (5)

GitHub Actions: CI / 0_✅ Tests Done.txt: feat(audio): persistent audio player across room navigation

Conclusion: failure

View job details

##[group]Run if [[ 'success' != 'success' ]]; then
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'failure' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mif [[ 'success' != 'success' ]]; then�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mecho finished�[0m
 shell: /usr/bin/bash -e {0}
 env:
   TOOL_NODE_FLAGS: --max_old_space_size=4096
 ##[endgroup]
 ##[error]Process completed with exit code 1.

GitHub Actions: CI / 6_📦 Track Image Sizes.txt: feat(audio): persistent audio player across room navigation

Conclusion: failure

View job details

##[group]Run current_total=$(jq -r '.total' current-sizes.json)
 �[36;1mcurrent_total=$(jq -r '.total' current-sizes.json)�[0m
 �[36;1m�[0m
 �[36;1mif [[ ! -f baseline-sizes.json ]]; then�[0m
 �[36;1m  echo "No baseline available"�[0m
 �[36;1m  echo "size-diff=0" >> $GITHUB_OUTPUT�[0m
 �[36;1m  echo "size-diff-percent=0" >> $GITHUB_OUTPUT�[0m
 �[36;1m  echo "comment-triggered=false" >> $GITHUB_OUTPUT�[0m
 �[36;1m�[0m
 �[36;1m  cat > report.md << 'EOF'�[0m
 �[36;1m# 📦 Docker Image Size Report�[0m
 �[36;1m�[0m
 �[36;1m**Status:** First measurement - no baseline for comparison�[0m
 �[36;1m�[0m
 �[36;1m**Total Size:** $(numfmt --to=iec-i --suffix=B $current_total)�[0m
 �[36;1mEOF�[0m
 �[36;1m  exit 0�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mbaseline_total=$(jq -r '.total' baseline-sizes.json)�[0m
 �[36;1mdiff=$((current_total - baseline_total))�[0m
 �[36;1m�[0m
 �[36;1mif [[ $baseline_total -gt 0 ]]; then�[0m
 �[36;1m  percent=$(awk "BEGIN {printf \"%.2f\", ($diff / $baseline_total) * 100}")�[0m
 �[36;1melse�[0m
 �[36;1m  percent=0�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mecho "size-diff=$diff" >> $GITHUB_OUTPUT�[0m
 �[36;1mecho "size-diff-percent=$percent" >> $GITHUB_OUTPUT�[0m
 �[36;1m�[0m
 �[36;1m# Only comment when size is bigger than baseline; optionally require per-image thresholds�[0m
 �[36;1mTHRESHOLDS="$SIZE_THRESHOLDS"�[0m
 �[36;1mFAIL_THRESHOLDS="$FAIL_THRESHOLDS"�[0m
 �[36;1mcomment_triggered=false�[0m
 �[36;1mfail_triggered=false�[0m
 �[36;1mif [[ $diff -gt 0 ]]; then�[0m
 �[36;1m  if [[ -z "$THRESHOLDS" ]] || [[ "$THRESHOLDS" == "{}" ]]; then�[0m
 �[36;1m    comment_triggered=true�[0m
 �[36;1m  fi�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mcolor="gray"�[0m
 �[36;1mif (( $(awk "BEGIN {print ($percent > 0.01)}") )); then�[0m
 �[36;1m  color="red"�[0m
 �[36;1melif (( $(awk "BEGIN {print ($percent < -0.01)}") )); then�[0m
 �[36;1m  color="green"�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1m# Generate report�[0m
 �[36;1mif [[ $diff -gt 0 ]]; then�[0m
 �[36;1m  emoji=...

GitHub Actions: CI / 13_🔨 Test UI (EE) _ MongoDB 8.0 coverage (5_5).txt: feat(audio): persistent audio player across room navigation

Conclusion: failure

View job details

##[group]Run set -o xtrace
 �[36;1mset -o xtrace�[0m
 �[36;1m�[0m
 �[36;1myarn prepare�[0m
 �[36;1myarn test:e2e --shard="$E2E_SHARD/$E2E_TOTAL_SHARD"�[0m
 shell: /usr/bin/bash -e {0}
 env:
   MONGO_URL: mongodb://localhost:27017/rocketchat?replicaSet=rs0&directConnection=true
   TOOL_NODE_FLAGS: --max_old_space_size=4096
   LOWERCASE_REPOSITORY: rocketchat
   DOCKER_TAG: pr-41120-amd64
   DOCKER_TAG_SUFFIX_ROCKETCHAT:
   MONGODB_VERSION: 8.0
   COVERAGE_DIR: /tmp/coverage/ui
   COVERAGE_FILE_NAME: ui-5.json
   COVERAGE_REPORTER: json
   NODE_VERSION: 22.22.3
   DENO_VERSION: 2.3.1
   MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
   MONGOMS_PREFER_GLOBAL_PATH: false
   TURBOGHA_PORT: 41230
   TURBO_API: http://localhost:41230
   TURBO_***REDACTED***
   TURBO_TEAM: turbogha
   E2E_COVERAGE: true
   IS_EE: true
   REPORTER_ROCKETCHAT_***REDACTED***
   REPORTER_ROCKETCHAT_URL: ***
   REPORTER_JIRA_ROCKETCHAT_***REDACTED***
   REPORTER_ROCKETCHAT_REPORT: true
   REPORTER_ROCKETCHAT_RUN: 62995
   REPORTER_ROCKETCHAT_BRANCH: refs/pull/41120/merge
   REPORTER_ROCKETCHAT_DRAFT: true
   REPORTER_ROCKETCHAT_HEAD_SHA: 6f1328704549e45109ba8b45b1f60a5acc9cdbb0
   REPORTER_ROCKETCHAT_AUTHOR: ggazzo
   REPORTER_ROCKETCHAT_RUN_URL: https://github.com/RocketChat/Rocket.Chat/actions/runs/28522795519
   REPORTER_ROCKETCHAT_PR: 41120
   QASE_API_***REDACTED***
   QASE_REPORT:
   CI: true
   PLAYWRIGHT_RETRIES: 0
   E2E_SHARD: 5
   E2E_TOTAL_SHARD: 5
 ##[endgroup]
 + yarn prepare
 + yarn test:e2e --shard=5/5
 GitCommitInfo: timeout of 3000ms exceeded while running "git fetch origin b6e0d3b30d86a01764df2dd652f989c20a51a11f"
 Running 141 tests using 1 worker, shard 5 of 5
   ✓  1 tests/e2e/rooms-join.spec.ts:46:7 › Join rooms › public channels without preview-c-room › should let a non-member join a public channel (7.7s)
 [INFO] qase: Test should let a non-member join a public channel passed
   ✓  2 tests/e2e/rooms-join.spec.ts:82:7 › Join rooms › public channel wi...

GitHub Actions: CI / 31_🔨 Test Unit _ Unit Tests.txt: feat(audio): persistent audio player across room navigation

Conclusion: failure

View job details

_old_space_size=4096
   ENTERPRISE_LICENSE: MK+bpK5NveUuNlWGaQXGoy+8b74Luet82M3ZGcBB8b5P9Y+m67NEtpW64dc1d5lEWi6d0nFjCjtCMneVD7bKxodz/Cml8URKEo5P7cQb/9wmeT0MzAhYNaRFZlIGkZ3ITF59pDV2u4HZuosEDJikVRwnaJ5ZoU/pOsHSPUPhTyGNIqLeKynODtUpfwDdIKEmHxpf2yVkKjgRiIJmbWjM6A4k+MNNYXWVXHzye7GggqWVg/ZcT7nKU1CCadpLhTJiIrgrrPzil1G5DQ4xnLs3Q2tu2dILSDiW5OYw/ywu2yCMicTjMq4MLL5SXDQJj6WoJzZ54HosbvsDzOXvsdC9gI1CjhPL2uRuvC8XLrzn3vL2UgXnifzD1VrLTtdZ+aSADveqtlzYlRWtqoUFBbNw8o+YVHdhbZGR0beMoAyRbHi5EMpxpad3L+NyztUIT/Uh/IjQ/C2SQZ6jB0GKPBOPxFLN56FNhTGrffLFR++TVoBu0Iquc7kajWkNit3bVbZvbx+oFcVW2PcjQ/+i2jpJjbgtUFUKrTKxGMAXTWoDzIQQ35zNzGAy268IM4Ymp5JmsVEnBOEUkbF9yx6fzkO6xZhpsHf0muklnW0kA+Tlore/TUrBWh1/RwWlQeZlxM5NyWoRM5onQmr/k/4BmObtL1Hpmbk8oMG29z89xtE9y/4=
   NODE_VERSION: 22.22.3
   DENO_VERSION: 2.3.1
   MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
   MONGOMS_PREFER_GLOBAL_PATH: false
   TURBOGHA_PORT: 41230
   TURBO_API: http://localhost:41230
   TURBO_***REDACTED***
   TURBO_TEAM: turbogha
 ##[endgroup]
 ##[group]Run missing_deps=""
 �[36;1mmissing_deps=""�[0m
 �[36;1m�[0m
 �[36;1m# Check for always-required commands�[0m
 �[36;1mfor cmd in bash git curl; do�[0m
 �[36;1m  if ! command -v "$cmd" >/dev/null 2>&1; then�[0m
 �[36;1m    missing_deps="$missing_deps $cmd"�[0m
 �[36;1m  fi�[0m
 �[36;1mdone�[0m
 �[36;1m�[0m
 �[36;1m# Check for gpg only if validation is not being skipped�[0m
 �[36;1mif [ "$INPUT_SKIP_VALIDATION" != "true" ]; then�[0m
 �[36;1m  if ! command -v gpg >/dev/null 2>&1; then�[0m
 �[36;1m    missing_deps="$missing_deps gpg"�[0m
 �[36;1m  fi�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1m# Report missing required dependencies�[0m
 �[36;1mif [ -n "$missing_deps" ]; then�[0m
 �[36;1m  echo "Error: The following required dependencies are missing:$missing_deps"�[0m
 �[36;1m  echo "Please install these dependencies before using this action."�[0m
 �[36;1m  exit 1�[0m
 �[36;1mfi�[0m
 �[36;1m�[0m
 �[36;1mecho "All required system dependencies are available."�[0m
 shell: /usr...

GitHub Actions: CI / 16_🔨 Test UI (EE) _ MongoDB 8.0 coverage (4_5).txt: feat(audio): persistent audio player across room navigation

Conclusion: failure

View job details

##[group]Run set -o xtrace
 �[36;1mset -o xtrace�[0m
 �[36;1m�[0m
 �[36;1myarn prepare�[0m
 �[36;1myarn test:e2e --shard="$E2E_SHARD/$E2E_TOTAL_SHARD"�[0m
 shell: /usr/bin/bash -e {0}
 env:
   MONGO_URL: mongodb://localhost:27017/rocketchat?replicaSet=rs0&directConnection=true
   TOOL_NODE_FLAGS: --max_old_space_size=4096
   LOWERCASE_REPOSITORY: rocketchat
   DOCKER_TAG: pr-41120-amd64
   DOCKER_TAG_SUFFIX_ROCKETCHAT:
   MONGODB_VERSION: 8.0
   COVERAGE_DIR: /tmp/coverage/ui
   COVERAGE_FILE_NAME: ui-4.json
   COVERAGE_REPORTER: json
   NODE_VERSION: 22.22.3
   DENO_VERSION: 2.3.1
   MONGOMS_DOWNLOAD_DIR: /home/runner/work/_temp/mongodb-memory-server
   MONGOMS_PREFER_GLOBAL_PATH: false
   TURBOGHA_PORT: 41230
   TURBO_API: http://localhost:41230
   TURBO_***REDACTED***
   TURBO_TEAM: turbogha
   E2E_COVERAGE: true
   IS_EE: true
   REPORTER_ROCKETCHAT_***REDACTED***
   REPORTER_ROCKETCHAT_URL: ***
   REPORTER_JIRA_ROCKETCHAT_***REDACTED***
   REPORTER_ROCKETCHAT_REPORT: true
   REPORTER_ROCKETCHAT_RUN: 62995
   REPORTER_ROCKETCHAT_BRANCH: refs/pull/41120/merge
   REPORTER_ROCKETCHAT_DRAFT: true
   REPORTER_ROCKETCHAT_HEAD_SHA: 6f1328704549e45109ba8b45b1f60a5acc9cdbb0
   REPORTER_ROCKETCHAT_AUTHOR: ggazzo
   REPORTER_ROCKETCHAT_RUN_URL: https://github.com/RocketChat/Rocket.Chat/actions/runs/28522795519
   REPORTER_ROCKETCHAT_PR: 41120
   QASE_API_***REDACTED***
   QASE_REPORT:
   CI: true
   PLAYWRIGHT_RETRIES: 0
   E2E_SHARD: 4
   E2E_TOTAL_SHARD: 5
 ##[endgroup]
 + yarn prepare
 + yarn test:e2e --shard=4/5
 GitCommitInfo: timeout of 3000ms exceeded while running "git fetch origin b6e0d3b30d86a01764df2dd652f989c20a51a11f"
 Running 149 tests using 1 worker, shard 4 of 5
   ✓  1 tests/e2e/omnichannel/omnichannel-livechat-hide-expand-chat.spec.ts:46:6 › OC - Livechat - Hide "Expand chat" › OC - Livechat - Hide "Expand chat" (7.0s)
 [INFO] qase: Test OC - Livechat - Hide "Expand chat" passed
   ✓  2 tests/e2e/omnichannel/omnichannel-livechat-logo.spec.ts:38:6 › OC ...
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}

📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)

**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation

Files:

  • apps/meteor/client/sidebar/Sidebar.tsx
  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/views/navigation/sidebar/Sidebar.tsx
  • apps/meteor/client/providers/MeteorProvider.tsx
  • apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
  • apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx
  • apps/meteor/client/sidebar/sections/SidebarCard.tsx
  • apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx
  • apps/meteor/client/components/message/content/Attachments.tsx
  • apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
  • apps/meteor/client/components/message/content/attachments/FileAttachment.tsx
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
  • apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
🧠 Learnings (7)
📚 Learning: 2026-03-27T14:52:56.865Z
Learnt from: dougfabris
Repo: RocketChat/Rocket.Chat PR: 39892
File: apps/meteor/client/views/room/contextualBar/Threads/Thread.tsx:150-155
Timestamp: 2026-03-27T14:52:56.865Z
Learning: In Rocket.Chat, there are two different `ModalBackdrop` components with different prop APIs. During review, confirm the import source: (1) `rocket.chat/fuselage` `ModalBackdrop` uses `ModalBackdropProps` based on `BoxProps` (so it supports `onClick` and other Box/DOM props) and does not have an `onDismiss` prop; (2) `rocket.chat/ui-client` `ModalBackdrop` uses a narrower props interface like `{ children?: ReactNode; onDismiss?: () => void }` and handles Escape keypress and outside mouse-up, and it does not forward arbitrary DOM props such as `onClick`. Flag mismatched props (e.g., `onDismiss` passed to the fuselage component or `onClick` passed to the ui-client component) and ensure the usage matches the correct component being imported.

Applied to files:

  • apps/meteor/client/sidebar/Sidebar.tsx
  • apps/meteor/client/views/navigation/sidebar/Sidebar.tsx
  • apps/meteor/client/providers/MeteorProvider.tsx
  • apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
  • apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx
  • apps/meteor/client/sidebar/sections/SidebarCard.tsx
  • apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx
  • apps/meteor/client/components/message/content/Attachments.tsx
  • apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
  • apps/meteor/client/components/message/content/attachments/FileAttachment.tsx
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
  • apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
📚 Learning: 2026-05-06T12:21:44.083Z
Learnt from: juliajforesti
Repo: RocketChat/Rocket.Chat PR: 40256
File: apps/meteor/client/components/CreateDiscussion/CreateDiscussion.tsx:121-149
Timestamp: 2026-05-06T12:21:44.083Z
Learning: Field wrappers in rocket.chat/fuselage-forms (Field, FieldLabel, FieldRow, FieldError, FieldHint) auto-create htmlFor/id associations, aria-describedby, and role="alert" for errors. Do not manually set htmlFor, id, aria-describedby, or role attributes when using these wrappers. This automatic wiring does not apply to plain rocket.chat/fuselage components, which require explicit ID wiring per the accessibility docs. In code reviews, prefer using fuselage-forms wrappers for form fields and verify there is no unnecessary manual ID/aria wiring in files that use these wrappers. If a component uses plain fuselage components, ensure proper id wiring as per docs.

Applied to files:

  • apps/meteor/client/sidebar/Sidebar.tsx
  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/views/navigation/sidebar/Sidebar.tsx
  • apps/meteor/client/providers/MeteorProvider.tsx
  • apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
  • apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx
  • apps/meteor/client/sidebar/sections/SidebarCard.tsx
  • apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx
  • apps/meteor/client/components/message/content/Attachments.tsx
  • apps/meteor/client/components/AudioPlayer/AudioPlayerControls.tsx
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx
  • apps/meteor/client/components/message/content/attachments/FileAttachment.tsx
  • apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
  • apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx
📚 Learning: 2026-02-10T16:32:42.586Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 38528
File: apps/meteor/client/startup/roles.ts:14-14
Timestamp: 2026-02-10T16:32:42.586Z
Learning: In Rocket.Chat's Meteor client code, DDP streams use EJSON and Date fields arrive as Date objects; do not manually construct new Date() in stream handlers (for example, in sdk.stream()). Only REST API responses return plain JSON where dates are strings, so implement explicit conversion there if needed. Apply this guidance to all TypeScript files under apps/meteor/client to ensure consistent date handling in DDP streams and REST responses.

Applied to files:

  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-05-11T20:30:35.265Z
Learnt from: tassoevan
Repo: RocketChat/Rocket.Chat PR: 40480
File: apps/meteor/client/meteor/startup/accounts.ts:59-61
Timestamp: 2026-05-11T20:30:35.265Z
Learning: In Rocket.Chat’s Meteor client code, when calling `dispatchToastMessage` with `{ type: 'error' }`, pass the raw caught error object as `message` without manual normalization. `dispatchToastMessage` is designed to accept `message: unknown` for error toasts, so avoid converting errors to strings (e.g., `String(error)`) or extracting `error.message` before passing them.

Applied to files:

  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In the Rocket.Chat repository, do not reference Biome lint rules in code review feedback. Biome is not used even if biome.json exists; only reference Biome rules if there is explicit, project-wide usage documented. For TypeScript files, review lint implications without Biome guidance unless the project enables Biome rules.

Applied to files:

  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-02-26T19:25:44.063Z
Learnt from: gabriellsh
Repo: RocketChat/Rocket.Chat PR: 38778
File: packages/ui-voip/src/providers/useMediaSession.ts:192-192
Timestamp: 2026-02-26T19:25:44.063Z
Learning: In this repository (RocketChat/Rocket.Chat), Biome lint rules are not used even if a biome.json exists. When reviewing TypeScript files (e.g., packages/ui-voip/src/providers/useMediaSession.ts), ensure lint suggestions do not reference Biome-specific rules. Rely on general ESLint/TypeScript lint rules and project conventions instead.

Applied to files:

  • apps/meteor/client/components/AudioPlayer/formatPlaybackTime.ts
  • apps/meteor/client/providers/MediaPlayerProvider/index.ts
  • apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerContext.ts
📚 Learning: 2026-03-16T21:50:37.589Z
Learnt from: amitb0ra
Repo: RocketChat/Rocket.Chat PR: 39676
File: .changeset/migrate-users-register-openapi.md:3-3
Timestamp: 2026-03-16T21:50:37.589Z
Learning: For changes related to OpenAPI migrations in Rocket.Chat/OpenAPI, when removing endpoint types and validators from rocket.chat/rest-typings (e.g., UserRegisterParamsPOST, /v1/users.register) document this as a minor changeset (not breaking) per RocketChat/Rocket.Chat-Open-API#150 Rule 7. Note that the endpoint type is re-exposed via a module augmentation .d.ts in the consuming package (e.g., packages/web-ui-registration/src/users-register.d.ts). In reviews, ensure the changeset clearly states: this is a non-breaking change, the major version should not be bumped, and the changeset reflects a minor version bump. Do not treat this as a breaking change during OpenAPI migrations.

Applied to files:

  • .changeset/persistent-audio-player.md
🔇 Additional comments (12)
.changeset/persistent-audio-player.md (1)

6-6: LGTM!

packages/i18n/src/locales/en.i18n.json (1)

3951-3951: LGTM!

Also applies to: 4220-4220, 4862-4862

apps/meteor/client/sidebar/sections/SidebarCard.tsx (1)

1-5: LGTM!

Also applies to: 10-16

apps/meteor/client/sidebar/sections/NowPlayingSection.tsx (1)

19-42: LGTM!

Also applies to: 52-76

apps/meteor/client/sidebar/Sidebar.tsx (1)

9-9: LGTM!

Also applies to: 25-25

apps/meteor/client/views/navigation/sidebar/Sidebar.tsx (1)

8-8: LGTM!

Also applies to: 17-17

apps/meteor/client/components/message/content/Attachments.tsx (1)

4-15: LGTM!

apps/meteor/client/components/message/content/attachments/AttachmentsItem.tsx (1)

8-18: LGTM!

apps/meteor/client/components/message/content/attachments/FileAttachment.tsx (1)

4-15: LGTM!

apps/meteor/client/components/message/content/attachments/file/AudioAttachment.tsx (1)

31-81: LGTM!

apps/meteor/client/components/message/variants/room/RoomMessageContent.tsx (1)

76-82: LGTM!

apps/meteor/client/components/message/variants/thread/ThreadMessageContent.tsx (1)

68-74: LGTM!

Comment thread apps/meteor/client/sidebar/sections/NowPlayingSection.tsx

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

5 issues found across 18 files

Prompt for AI agents (unresolved issues)

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


<file name="apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx">

<violation number="1" location="apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx:74">
P2: If the recovery URL (fetched during `handleMediaURLRecovery`) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the `loadedmetadata` event never fires, so `isRecoveringRef` stays `true` permanently. All subsequent `error`/`stalled` events for that track are silently dropped, and no further recovery is ever attempted. The `catch` block only catches synchronous errors and promise rejections from `getRedirectURLInfo`, not the asynchronous loading failure after `node.load()`. Consider resetting `isRecoveringRef` when loading the new URL also errors out, so a later retry can still recover.</violation>
</file>

<file name="apps/meteor/client/sidebar/sections/SidebarCard.tsx">

<violation number="1" location="apps/meteor/client/sidebar/sections/SidebarCard.tsx:10">
P2: Missing `memo` wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with `memo`, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.</violation>
</file>

<file name="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx">

<violation number="1" location="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx:34">
P2: If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because `useGoToRoom` handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale `msg` query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing `goToRoom` to expose failure to this caller.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx Outdated
if (isRecoveringRef.current) {
return;
}
isRecoveringRef.current = true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: If the recovery URL (fetched during handleMediaURLRecovery) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the loadedmetadata event never fires, so isRecoveringRef stays true permanently. All subsequent error/stalled events for that track are silently dropped, and no further recovery is ever attempted. The catch block only catches synchronous errors and promise rejections from getRedirectURLInfo, not the asynchronous loading failure after node.load(). Consider resetting isRecoveringRef when loading the new URL also errors out, so a later retry can still recover.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/providers/MediaPlayerProvider/MediaPlayerProvider.tsx, line 74:

<comment>If the recovery URL (fetched during `handleMediaURLRecovery`) also fails to load — for example the redirect URL is also expired or there is a transient network issue — the `loadedmetadata` event never fires, so `isRecoveringRef` stays `true` permanently. All subsequent `error`/`stalled` events for that track are silently dropped, and no further recovery is ever attempted. The `catch` block only catches synchronous errors and promise rejections from `getRedirectURLInfo`, not the asynchronous loading failure after `node.load()`. Consider resetting `isRecoveringRef` when loading the new URL also errors out, so a later retry can still recover.</comment>

<file context>
@@ -0,0 +1,219 @@
+		if (isRecoveringRef.current) {
+			return;
+		}
+		isRecoveringRef.current = true;
+
+		try {
</file context>

* Reusable elevated surface for sidebar sections: a padded, rounded, subtly
* bordered container in the sidebar tint.
*/
const SidebarCard = ({ children }: SidebarCardProps) => (

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: Missing memo wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with memo, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/SidebarCard.tsx, line 10:

<comment>Missing `memo` wrapper inconsistent with sidebar convention. Every other exported component in the sidebar directory wraps with `memo`, which helps prevent unnecessary re-renders of this pure presentational card when the parent re-renders.</comment>

<file context>
@@ -0,0 +1,16 @@
+ * Reusable elevated surface for sidebar sections: a padded, rounded, subtly
+ * bordered container in the sidebar tint.
+ */
+const SidebarCard = ({ children }: SidebarCardProps) => (
+	<Box m={8} p={10} borderRadius='x8' borderWidth='default' borderStyle='solid' borderColor='stroke-light' backgroundColor='surface-tint'>
+		{children}
</file context>

Comment thread apps/meteor/client/sidebar/sections/NowPlayingSection.tsx
if (!track.rid) {
return;
}
await goToRoom(track.rid);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P2: If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because useGoToRoom handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale msg query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing goToRoom to expose failure to this caller.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/NowPlayingSection.tsx, line 34:

<comment>If opening the source room fails, the Now Playing jump action still adds the audio message id to the current URL because `useGoToRoom` handles errors internally and returns normally. That can leave the user on the wrong conversation with a stale `msg` query parameter; consider only setting the jump parameter after a confirmed navigation target, or changing `goToRoom` to expose failure to this caller.</comment>

<file context>
@@ -0,0 +1,78 @@
+		if (!track.rid) {
+			return;
+		}
+		await goToRoom(track.rid);
+		if (track.mid) {
+			void setMessageJumpQueryStringParameter(track.mid);
</file context>

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

1 issue found across 2 files (changes from recent commits).

Prompt for AI agents (unresolved issues)

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


<file name="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx">

<violation number="1" location="apps/meteor/client/sidebar/sections/NowPlayingSection.tsx:48">
P3: Tracks that do not have `rid` now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since `openConversation()` returns early for those tracks, it would be better to apply `role`, `tabIndex`, and keyboard/click handlers only when `track.rid` is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.</violation>
</file>

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment on lines +48 to +56
role='button'
tabIndex={0}
onClick={() => void openConversation()}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
void openConversation();
}
}}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

P3: Tracks that do not have rid now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since openConversation() returns early for those tracks, it would be better to apply role, tabIndex, and keyboard/click handlers only when track.rid is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.

Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At apps/meteor/client/sidebar/sections/NowPlayingSection.tsx, line 48:

<comment>Tracks that do not have `rid` now expose the Now Playing header as a focusable button even though activating it cannot navigate anywhere. Since `openConversation()` returns early for those tracks, it would be better to apply `role`, `tabIndex`, and keyboard/click handlers only when `track.rid` is present (or mark the control disabled) so keyboard and screen-reader users don't land on a dead button.</comment>

<file context>
@@ -45,7 +45,15 @@ const NowPlayingSection = () => {
 					alignItems='center'
 					minWidth={0}
 					flexGrow={1}
+					role='button'
+					tabIndex={0}
 					onClick={() => void openConversation()}
</file context>
Suggested change
role='button'
tabIndex={0}
onClick={() => void openConversation()}
onKeyDown={(e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
void openConversation();
}
}}
role={track.rid ? 'button' : undefined}
tabIndex={track.rid ? 0 : undefined}
onClick={track.rid ? () => void openConversation() : undefined}
onKeyDown={
track.rid
? (e) => {
if (e.key === 'Enter' || e.key === ' ') {
e.preventDefault();
void openConversation();
}
}
: undefined
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature Pull requests that introduces new feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant