Skip to content

fix(mothership): hang condition ux bug#3852

Merged
icecrasher321 merged 1 commit intostagingfrom
fix/mothership-hang-condition
Mar 30, 2026
Merged

fix(mothership): hang condition ux bug#3852
icecrasher321 merged 1 commit intostagingfrom
fix/mothership-hang-condition

Conversation

@icecrasher321
Copy link
Copy Markdown
Collaborator

Summary

When stream hangs, even after stop and resend of message -- it queues it. This PR fixes that by releasing lock accurately.

Type of Change

  • Bug fix

Testing

Tested manually

Checklist

  • Code follows project style guidelines
  • Self-reviewed my changes
  • Tests added/updated and passing
  • No new warnings introduced
  • I confirm that I have read and agree to the terms outlined in the Contributor License Agreement (CLA)

@vercel
Copy link
Copy Markdown

vercel bot commented Mar 30, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
docs Skipped Skipped Mar 30, 2026 11:16pm

Request Review

@cursor
Copy link
Copy Markdown

cursor bot commented Mar 30, 2026

PR Summary

Medium Risk
Adjusts chat-stream coordination and client recovery state; mistakes could leave streams stuck or drop queued messages, but scope is small and localized.

Overview
Fixes a hang/queueing edge case when a chat stream is stopped mid-response.

The stop endpoint now calls releasePendingChatStream(chatId, streamId) before persisting partial output, ensuring any pending stream lock/state is cleared. On the client, useChat now clears pendingRecoveryMessage when finalizing with an error so failed recoveries don’t leave a stale message that keeps re-queuing.

Written by Cursor Bugbot for commit dddbf53. Configure here.

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 30, 2026

Greptile Summary

This PR fixes a deadlock/hang condition in the Mothership copilot chat: when a stream hangs, stopping it and immediately resending a message would leave the new message stuck in the queue because the per-chat distributed lock (Redis + in-memory promise) was never released until the hung stream's own finally block ran. Two targeted fixes address this:

  • Server-side (route.ts): The stop API now calls releasePendingChatStream(chatId, streamId) immediately upon receiving a stop request, freeing both the Redis lock and the in-memory pendingChatStreams entry. The underlying releaseLock Lua script is atomic and compare-and-delete safe, so double-release (from the SSE stream's own finally) is a no-op.
  • Client-side (use-chat.ts): The finalize error branch now also clears pendingRecoveryMessageRef and pendingRecoveryMessage state, fixing a stale-UI case where a recovery message would remain displayed/tracked after the stream errored out.

Key concern: The lock is released before the DB update in the stop route. If a new stream starts and writes conversationId in the small window before the DB update executes, the WHERE conversationId = streamId clause will miss and the partial stopped-stream content won't be persisted. Moving the releasePendingChatStream call to after the DB update would close this gap with no functional downside.

Confidence Score: 4/5

  • Safe to merge after reviewing the lock-release ordering; the partial-content loss window is narrow but real.
  • Both changes are small and targeted. The client-side fix is clean. The server-side fix correctly resolves the hang bug, but the ordering of releasePendingChatStream before the DB update introduces a narrow race where stopped-stream partial content could be silently dropped. This is a P2 data-integrity concern; no P0/P1 correctness bugs.
  • apps/sim/app/api/mothership/chat/stop/route.ts — verify lock-release ordering relative to DB update

Important Files Changed

Filename Overview
apps/sim/app/api/mothership/chat/stop/route.ts Adds releasePendingChatStream call to proactively free the distributed lock when a user stops a stream, fixing the hang/queue bug. The call is placed before the DB update, which creates a narrow race window where partial content may not be persisted if a new stream starts immediately.
apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Clears pendingRecoveryMessageRef and pendingRecoveryMessage state in the error branch of finalize, preventing the recovery message from persisting in UI state after a stream error.

Sequence Diagram

sequenceDiagram
    participant C as Client (useChat)
    participant S as Stop API Route
    participant L as Lock (Redis + pendingChatStreams)
    participant D as Database
    participant N as New Stream

    Note over C,N: Bug scenario (before fix)
    C->>S: POST /stop (streamId=A)
    S->>D: UPDATE copilotChats SET conversationId=null WHERE conversationId=A
    Note over L: Lock still held by stream A's finally block
    C->>N: sendMessage (new message)
    N-->>L: acquireLock() → timeout/queue (lock never released!)

    Note over C,N: Fixed flow (after this PR)
    C->>S: POST /stop (streamId=A)
    S->>L: releasePendingChatStream(chatId, A) ← NEW
    S->>D: UPDATE copilotChats SET conversationId=null WHERE conversationId=A
    C->>N: sendMessage (new message)
    N->>L: acquireLock() → acquired immediately ✓
    N->>D: INSERT new stream

    Note over C: Client-side fix (use-chat.ts)
    C-->>C: finalize({ error: true })
    C-->>C: pendingRecoveryMessageRef = null ← NEW
    C-->>C: setPendingRecoveryMessage(null) ← NEW
    C-->>C: setMessageQueue([])
Loading

Reviews (1): Last reviewed commit: "fix(mothership): hang condition" | Re-trigger Greptile

@icecrasher321 icecrasher321 merged commit c764319 into staging Mar 30, 2026
12 checks passed
@waleedlatif1 waleedlatif1 deleted the fix/mothership-hang-condition branch March 31, 2026 04:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant