fix(playground): clear stale pending-signal badge in Studio chat#17682
fix(playground): clear stale pending-signal badge in Studio chat#17682aisensiy wants to merge 1 commit into
Conversation
The "pending: …" indicator above the chat input could linger after the agent finished replying (and pile up across messages), clearing only on a page refresh. The badge is added via onSignalSent (the send-message HTTP response) and removed via onSignalEcho (the thread-subscription stream) — two async channels with no ordering guarantee. On the idle path the echo arrives before the add, so the remove ran as a no-op and the late add orphaned a badge that no further echo would ever clear. Make the bookkeeping order-independent using synchronous id mirrors (pendingSignalIdsRef + prematurelyEchoedSignalIdsRef) so add and remove are commutative, routed through a clearPendingSignals() helper at the reset sites. Also from review follow-up: add afterEach(cleanup) to the provider test (this package runs vitest with globals disabled, so React Testing Library's auto-cleanup never registers and rendered providers leaked between tests), harden getSignalCallbacks, and reuse the exported PendingSignalMessage type. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@aisensiy is attempting to deploy a commit to the Mastra Team on Vercel. A member of the Team first needs to authorize it. |
🦋 Changeset detectedLatest commit: d85a977 The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 |
WalkthroughThis PR fixes a race condition where "pending" signal badges linger in Studio chat when the server's signal echo arrives before the send confirmation. The fix introduces commutative ref-based tracking in ChangesPending signal race-condition fix
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
PR triageLinked issue check passed (#17681). Mastra uses CodeRabbit for automated code reviews. Please address all feedback from CodeRabbit by either making changes to your PR or leaving a comment explaining why you disagree with the feedback. Since CodeRabbit is an AI, it may occasionally provide incorrect feedback. PR complexity score
Applied label: Changed test gateChanged Test Gate is pending. The |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/playground/src/services/__tests__/mastra-runtime-provider.test.tsx (1)
237-319: 🏗️ Heavy liftpotential_issue: New signal lifecycle tests continue the mocked-callback pattern instead of the package’s MSW-first integration path.
These cases validate
onSignalSent/onSignalEchovia mockeduseChatinternals rather than exercising the real@mastra/client-js+ React Query flow with network-only mocking.As per coding guidelines:
packages/playground/**/*.test.{ts,tsx}should use the MSW-first strategy and “never mock playground’s own data hooks, services, or auth gating with vi.mock.”Source: Coding guidelines
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1567e557-eaf7-4f7d-a5d3-a066c6ccd66a
📒 Files selected for processing (3)
.changeset/studio-pending-signal-badge.mdpackages/playground/src/services/__tests__/mastra-runtime-provider.test.tsxpackages/playground/src/services/mastra-runtime-provider.tsx
Closes #17681
Summary
Studio chat's
pending: …indicator above the message input could linger after the agent finished replying — and pile up across messages — clearing only on a page refresh.Root cause
The badge is added via
onSignalSent(the send-message HTTP response) and removed viaonSignalEcho(the thread-subscription stream) — two async channels with no ordering guarantee. On the idle path the server starts the run and emits thedata-user-messageecho over the already-open subscription before the send-message response returns, soonSignalEchofired beforeonSignalSenthad added the pending entry: the remove ran as a no-op, then the late add orphaned a badge that no further echo would ever clear. (The echo itself arrives fine — the user message renders in history — only the badge is stranded.)Fix
Make the pending bookkeeping order-independent using two synchronous id mirrors, so add/remove are commutative regardless of arrival order:
pendingSignalIdsRef— mirrors the currently-visible badge idsprematurelyEchoedSignalIdsRef— remembers echoes that landed before their add, so the late add is skipped instead of resurrecting a badgeBoth are routed through a
clearPendingSignals()helper at the three reset sites (thread/agent switch,onThreadSignalsUnsupported,onCancel).Testing
pendingno longer appears.pnpm --filter ./packages/playground test src/services→ 54/54, plus typecheck and eslint pass.Review follow-ups included here
afterEach(cleanup)to the provider test — this package runs vitest with globals disabled, so React Testing Library's auto-cleanup never registers and rendered providers were leaking between tests; also hardenedgetSignalCallbacks.PendingSignalMessagetype instead of an inline literal.Known follow-ups (not in this PR)
A fully-robust fix for two related, lower-severity edges — a narrow reset-vs-late-send re-race, and unbounded growth of the premature-echo set from foreign/duplicate echoes on a shared thread — belongs at the
@mastra/reactuseChatSDK boundary, where the ordering hazard originates (both callbacks fire from inside the hook). Tracking separately rather than band-aiding per consumer.🤖 Generated with Claude Code
ELI5
When you send a message in Studio chat, a "pending: ..." badge appears while the message is being processed. This badge should disappear once the server confirms the message was received, but sometimes it sticks around and won't go away even after you get a reply—you have to refresh the page to clear it. This happens because the confirmation and the acknowledgment can arrive out of order, and the code only knew how to clean up if they arrived in the right order.
Summary
This PR fixes a race condition in Studio's chat pending signal indicator where the
pending: …badge above the message input could linger after the agent replied and accumulate across messages.Root cause: The pending badge is added when the send-message HTTP response arrives (
onSignalSent) and removed when the echo is received from the thread-subscription stream (onSignalEcho). Since these two async channels have no ordering guarantee, when an echo arrives before the send response completes, the premature remove becomes a no-op. A subsequent add then leaves an orphaned badge that never clears.Solution: The provider now maintains order-independent bookkeeping using two synchronous
Setrefs:pendingSignalIdsRef: tracks currently-visible pending badge IDsprematurelyEchoedSignalIdsRef: tracks echoes that arrived before their corresponding send confirmationA new
clearPendingSignals()helper centralizes cleanup logic and ensures both refs are cleared at reset points (thread/agent switch, losing signal support, cancelling a run). The logic skips late adds if an echo was already seen for that signal ID, making the add/remove operations commutative regardless of arrival order.Changes:
mastra-runtime-provider.tsx: Added the two ref-based tracking sets andclearPendingSignals()helper; updated thread/agent switch, signal-unsupported handler, and cancel handlers to use centralized cleanupmastra-runtime-provider.test.tsx: Addedcleanup()between tests to prevent state leakage; added three new unit tests validating signal lifecycle under normal ordering, reverse ordering (echo-before-send), and per-ID independence.changeset/studio-pending-signal-badge.md: Documents the patch release for@internal/playgroundTesting: Deterministic unit tests now force the problematic echo-before-add ordering (previously failed, now pass), plus normal-order and per-id independence cases; validated live in dev Studio; package tests and linters pass.