feat(frontend): show stale progress when music SSE lags

[Adolanium]

Description

The dashboard progress bar uses state.position from SSE. If the stream lags, the bar freezes mid-track with no indication that the position is stale.

Fix

• useMusicPlayer: record lastStateUpdate (wall clock) on every successful SSE or REST state payload.
• Now-playing hero: while playing, re-tick every second; if no state update for 5s, dim the bar, switch it to warning color, and show a "may be outdated" label.

Verification

• Unit tests assert lastStateUpdate is set from SSE and initial REST state.

Checklist

• useMusicPlayer tests for lastStateUpdate
• CHANGELOG.md updated (if user-facing)
• TypeScript clean for touched files

Destructive / irreversible interaction (Tier A)

Not applicable. Display-only.

Feature-removal sweep

Not applicable.

Fixes #1772

---

Summary by cubic

Show a stale indicator on the now-playing progress bar when SSE updates or heartbeats lag. Marks progress outdated after 45s and announces it via role="status", addressing Linear #1772.

• New Features

  • Track lastStateUpdate from SSE, initial REST, and heartbeat in useMusicPlayer. While playing, tick every second; after 45s without a stamp, dim the bar, switch to warning, and show a centered, localized “may be outdated” label without shifting timestamps.

• Bug Fixes

  • Use shared applyState for SSE/REST and optimistic rollbacks so staleness clears on refresh; ignore malformed SSE. Server heartbeats now send data: {"type":"heartbeat"} and are treated as liveness only. Removed unused aria markup; stale notice is announced via role="status".

^{Written for commit 6823487. Summary will update on new commits.}

Summary by CodeRabbit

• New Features

  • Added detection for outdated music playback progress.
  • Progress indicators are dimmed and display a warning when updates have not been received recently.
  • Heartbeat updates now refresh playback freshness without changing the current queue state.
  • Added English and Portuguese translations for the outdated-progress message.

• Bug Fixes

  • Improved handling of malformed streaming messages without overwriting player state.

[cubic-dev-ai]

All reported issues were addressed across 3 files

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

Re-trigger cubic</sub>

[cubic-dev-ai]

0 issues found across 4 files (changes from recent commits).

<sub>Auto-approved: Display-only stale indicator for progress bar when SSE lags; no behavioral or operational changes, bounded frontend enhancement verified by tests.

Re-trigger cubic</sub>

[LucasSantana-Dev]

Kimi review (kimi-code/kimi-for-coding, via local subscription)

• VERDICT: CLEAN

No substantive concerns: state-update timestamping is straightforward, the interval effect is correctly cleaned up, dependency arrays are updated, and tests cover the new behavior. Existing swallowed .catch(() => {}) patterns are unchanged by this PR.

---

Head 7fb45c7. Posted by kimi-review-watch (launchd).

[LucasSantana-Dev]

I like the shape of this. Funnelling both the SSE and REST paths through a single applyState so there's exactly one place that stamps the timestamp is the right call, and gating the re-tick interval on state.isPlaying avoids leaving a timer running forever.

But I went and read the server side, and I think the warning will fire constantly on healthy connections. I'd like to sort that out before this goes in.

The staleness signal will show up when nothing is wrong

lastStateUpdate only advances when a payload parses as JSON state. Everything else lands in the catch:

try {
    applyState(JSON.parse(event.data))
} catch {
    /* heartbeat or malformed data */
}

So it depends entirely on how often the server pushes real state. Looking at packages/backend/src/routes/music/stateRoutes.ts:

• it writes the full state once on connect (line 27);
• after that the only periodic write is res.write(': heartbeat\n\n') on a 30 second interval (lines 44-59);
• actual state payloads only go out when something broadcasts to sseClients, i.e. on a real change.

Two problems with that. The heartbeat is an SSE comment (: prefix), so it carries no data: field and won't even reach onmessage, let alone refresh the timestamp. And the interval is 30s against STALE_AFTER_MS = 5_000, so it's six times too slow to help even if it did.

Net effect: start a track, don't touch anything, and 5 seconds later the bar dims to opacity-50, flips to bg-lucky-warning and says "may be outdated" while the connection is perfectly fine. It then stays that way until someone edits the queue. That's the opposite of the signal you want, and it'll teach people to ignore the warning.

A few ways out, your call which fits best:

• derive staleness from isConnected (the hook already tracks it) rather than payload recency;
• give the heartbeat a real data: payload so it parses and stamps the timestamp, which also makes it a genuine liveness signal;
• or keep the recency approach but set the threshold above the real push interval, which means >30s and coupling this constant to the server's heartbeat.

I'd lean toward the second, since a heartbeat that the client can't observe isn't doing much for us today anyway.

Smaller thing: the aria-label doesn't do anything

<div className='flex-1 h-1 ...' aria-label={isStale ? t('music.progressMayBeOutdatedAria') : undefined}>

A div has no implicit role, and aria-label on a roleless generic element is ignored by most screen readers. It's also redundant, since you already add the same information as visible text just below, which assistive tech picks up for free. Either drop it, or give the element role='progressbar' with aria-valuenow/aria-valuemin/aria-valuemax — which is what this control actually is, and would be a real improvement beyond the scope of this PR. Right now it's just dead markup.

Nit: putting the label between the position and duration spans turns that row from a two-item justify-between into three, so the timestamps stop sitting at the extremes and jump inward whenever the warning appears. Reserving the space or absolutely positioning the notice would avoid the shift.

Merge order: #1866 rewrites the same useMusicPlayer.ts region and also touches pages/Music.tsx and both locale files; #1864 touches Music.tsx too. I'll sequence the three rather than letting them collide.

On the red checks: not yours. Security was an unpassable repo-wide gate, kimi-review fails on every PR because an API key isn't set (#1877), and the npm ci errors come from a stale lockfile on main. Fixed in #1876; rebase once it lands.

[LucasSantana-Dev]

Heads-up: the CI blockers I mentioned are fixed on main now (#1876, merged as ca410d2c).

What that clears:

• Security — it ran npm audit --audit-level=high across devDependencies and could never pass, which is why it was red on every PR including this one. It now audits production deps only, with an explicit allowlist pinned to individual advisory ids.
• npm ci / Build — shared / Quality Gates — main's lockfile was stale and npm@12's stricter ci rejected it (lock file's eslint@10.7.0 does not satisfy eslint@10.8.0). Resynced.
• compressed-size — it installs the base branch too, so it was failing on main's lockfile rather than on your changes.
• kimi-review — removed in ci: remove kimi-review until an api key is set #1884; the API key was never set, so it failed on every PR in the repo.

Please rebase onto main and push. The auto-update workflow only touches branches whose authors enabled auto-merge, so it deliberately won't rewrite yours. A rebase should turn the checks green without any change to your code.

The review feedback above is separate and still stands.

[Adolanium]

Addressed in b53707f (squashed, rebased onto main).

• Stream heartbeat is now a real SSE data payload ({ type: 'heartbeat' }) instead of a comment, so it reaches onmessage.
• The client stamps lastStateUpdate on that heartbeat without touching queue state.
• STALE_AFTER_MS is 45s (1.5x the 30s server interval) so a healthy idle connection no longer flips to warning after 5s.
• Dropped the roleless aria-label on the progress div. Visible text still covers the stale case.
• Stale notice is absolutely centered so the position/duration timestamps stay at the edges.

[LucasSantana-Dev]

Review: approve with nits

Correct, well-scoped, and fixes a real protocol bug. Remaining items are a misleading PR description, one semantic question, and minor polish.

P2 — PR body vs code: the description says "if no state update for 5s, dim the bar", but STALE_AFTER_MS = 45_000 (Music.tsx:144). The code is the right call (5s would flap against the 30s server heartbeat), so the description is stale, not the code. Please update the PR body so reviewers and future git blame readers aren't chasing a 5s threshold that never existed.

P2 — question on the staleness definition: heartbeats stamp lastStateUpdate, so the stale indicator only fires when BOTH state broadcasts and heartbeats stop for 45s. If the #1772 failure mode is the backend stopping state publishes while the SSE connection (and its heartbeats) stays alive, the bar stays frozen with no indicator. Fine if the intended definition is "connection liveness" (per the code comment); if it's position-data freshness, heartbeats mask it.

P3 (nits): progressMayBeOutdatedAria is added to both locales but never referenced (dead key — drop it or wire it up); the stale flip is conveyed via title on a non-interactive div, which is keyboard/touch-inaccessible and not announced — the ConnectionBadge pattern (role='status') is the in-repo precedent; hook-level tests cover lastStateUpdate but nothing exercises the NowPlayingHero stale UI.

What's good: the protocol fix is real and correct — SSE comment lines (: heartbeat) never fire onmessage, so the new data: {"type":"heartbeat"} payload plus the payload?.type === 'heartbeat' branch is the only way the client can observe liveness, and QueueState has no type field so there's no collision with real broadcasts. Optimistic updates correctly bypass applyState, lastStateUpdate resets on guild change, and the 1s re-tick interval only runs while playing and is cleaned up.

[Adolanium]

Addressed the remaining nits.

• PR body now matches the code (STALE_AFTER_MS = 45_000, not 5s).
• Dropped the unused progressMayBeOutdatedAria / title keys.
• Stale notice uses role="status" so assistive tech picks it up without a dead title on a non-interactive div.
• Heartbeat stamping stays as connection liveness (per the code comment). Position freshness without heartbeats would need a different signal.

Rebased onto main.

[github-actions]

	Warnings
⚠️	User-facing change without a CHANGELOG.md update. Add a line under ## [Unreleased] if this should appear in release notes. (Or apply the skip-changelog label if this PR does not affect end users.)

Generated by 🚫 dangerJS against 6823487

[LucasSantana-Dev]

A note from the maintainer side: sorry this PR waited as long as it did for a proper review, and sorry for the rounds of branch updates and re-running checks today. The churn was on our side, not yours.

Your PRs exposed real gaps in how this repo handled external contributions: CI runs sat in a silent approval queue, some gates could never pass on fork PRs (SonarCloud, danger), and the team had no notification when external PRs arrived. Those are all fixed as of today:

• CI auto-approves for returning contributors, no more waiting on a manual click.
• All gates now pass on fork PRs (SonarCloud skips gracefully without a token, danger runs and comments correctly).
• The team gets notified the moment an external PR is opened.

Your branch is up to date and the full suite is green. Thanks for the patience and for the contribution. External contributors are very welcome here.

[coderabbitai]

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 `@packages/frontend/src/hooks/useMusicPlayer.ts`:
- Around line 99-107: Validate parsed WebSocket payloads in useMusicPlayer
before applyState, accepting only heartbeat messages or complete QueueState
objects and ignoring all other valid JSON values. In
packages/frontend/src/hooks/useMusicPlayer.ts lines 99-107, add the runtime
validation while preserving heartbeat handling; in
packages/frontend/src/hooks/useMusicPlayer.test.ts lines 200-204, add a
syntactically valid invalid payload such as {} and assert the previous queue
state remains unchanged.
- Around line 187-189: Update the successful optimistic command path in
useMusicPlayer so it applies the returned state through the same
liveness-stamping mechanism as the rollback refresh, rather than only calling
setState. Ensure the existing isLiveCommand guard and stale-progress behavior
remain intact while refreshing freshness immediately after a successful command.

🪄 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: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro Plus

Run ID: c474b2f6-f2b5-4991-a562-9762a625a4da

📥 Commits

Reviewing files that changed from the base of the PR and between 2dda60f and 6823487.

📒 Files selected for processing (6)

• packages/backend/src/routes/music/stateRoutes.ts
• packages/frontend/src/hooks/useMusicPlayer.test.ts
• packages/frontend/src/hooks/useMusicPlayer.ts
• packages/frontend/src/locales/en.json
• packages/frontend/src/locales/pt-BR.json
• packages/frontend/src/pages/Music.tsx