Skip to content

[Detail Bug] Outbound messages can remain stuck as "Sending" when delivery outcome is uncertain or dispatch lease expires #381

Description

@detail-app

Detail Bug Report

https://app.detail.dev/org_ee14aa77-b24a-40b2-b22d-66bd31931f4a/bugs/bug_98dbe0df-c6a1-40cf-a5ec-fc9e6e905e77

Introduced in #328 by @setkyar on Sep 9, 2026

Summary

  • Context: channel-outbound.service.ts is the neutral-path dispatcher that claims outbound_message_intents, hands the send to the channel adapter, and resolves the intent and the underlying messages row.
  • Bug: When an intent resolves to "uncertain", the service updates the intent's status but never updates the message row and never fires a fanout, so the message is stuck at "pending" with a perpetual "Sending" spinner. The same omission exists in recoverExpiredLeases, the lease-expiry recovery path. No compensating mechanism — the dispatch loop, late delivery receipts, or the recurring message-cleanup job — ever recovers these messages.
  • Actual vs. expected: A neutral-path send whose delivery outcome is unknown should surface the amber "unconfirmed" indicator the legacy path produces; instead it shows a perpetual "Sending" spinner with no recovery path of any kind.
  • Impact: A user who sends a message via the spine sees it spin forever as "Sending" if the provider returns an uncertain outcome (transport transaction failure) or its dispatch lease expires and the process restarts; the message is never marked unconfirmed or failed, no retry affordance is offered, and no background sweep cleans it up.

Code with Bug

In completeClaim, the uncertain branch is explicitly excluded from the message update + fanout:

// channel-outbound.service.ts:556-576
if (
  fenced.message_id &&
  !transient &&
  failure.outcome !== "uncertain" && // <-- BUG 🔴 uncertain excluded, so message row + fanout never updated for unknown outcome
  !claim.operation.startsWith("action:")
) {
  await trx
    .updateTable("messages")
    .set({ status: "failed" })
    .where("id", "=", fenced.message_id)
    .execute();
  await enqueueOutboundRealtimeFanout(
    trx,
    companyId,
    claim.channelAccountId,
    claim.conversationId,
    fenced.message_id,
  );
}

In recoverExpiredLeases, the intent is set to uncertain but the messages row is never updated and no fanout is emitted:

// channel-outbound.service.ts:588-599
await tenantDb
  .updateTable("outbound_message_intents")
  .set({
    status: "uncertain", // <-- BUG 🔴 message row left "pending" and no fanout, causing permanent spinner
    lease_token: null,
    lease_expires_at: null,
    last_error_code: "dispatch_lease_expired_outcome_unknown",
    updated_at: new Date(),
  })
  .where("status", "=", "dispatching")
  .where("lease_expires_at", "<", new Date())
  .execute();

The dispatch loop never reclaims uncertain (or dispatching) intents:

// channel-outbound.service.ts:199
.where("intent.status", "=", "pending") // <-- BUG 🔴 "uncertain"/"dispatching" intents never re-claimed, so they never resolve

Explanation

  • The neutral dispatcher only claims intents in pending. When an outbound attempt returns outcome: "uncertain" or a lease expires on restart, the intent moves to uncertain (or can remain dispatching after a graceful restart), but no code path updates the associated messages row to indicate an unknown outcome, and no realtime fanout is emitted.
  • uncertain becomes a sink state: the dispatcher does not re-claim it, and repository search found no transition from uncertain back to another terminal/retryable state.
  • Late delivery receipts cannot rescue these messages because the transport updates messages.message_id and enqueues the command in a single transaction; if it throws, that transaction rolls back and the code returns { outcome: "uncertain" }. The message’s message_id remains null, and receipt handlers key off message_id, so no receipt can match the stuck row.
  • The recurring message-cleanup job does not sweep these stuck rows: its reconciliation joins require m.message_id (which is null for uncertain), and its stale sweep requires m.metadata IS NULL, but neutral-path sends always set metadata to an object (never NULL).
  • Restart behavior creates an additional permanent sink: after a graceful restart before the lease expires, intents can remain dispatching, which also is never reclaimed by the dispatch loop.

Codebase Inconsistency

Legacy send-failure handling explicitly supports “outcome unknown” and the UI expects it:

// MessageStatusIcon.tsx
if (message.status === "pending" && message.metadata?.error === "send_outcome_unknown") {
  // amber "?", tooltip "Delivery unconfirmed. Check the conversation before sending again."
}

The neutral path never writes metadata.error = "send_outcome_unknown" and never fanouts on uncertain, so the UI renders a perpetual “Sending” spinner instead of the amber “?” indicator.

Recommended Fix

  • In completeClaim, add an uncertain branch for non-action intents that writes metadata.error = "send_outcome_unknown" onto the messages row and calls enqueueOutboundRealtimeFanout, mirroring the legacy behavior.
  • In recoverExpiredLeases, when marking intents uncertain, apply the same message metadata update and fanout for any associated message.
  • Also address the dispatching sink on graceful restarts by allowing reclaim of expired dispatching intents (either by broadening the dispatch query or making recovery handle dispatching intents whose leases have expired at runtime).

History

This bug was introduced in commit f1a440b. This is the original "Channel-neutral messaging spine with Telegram Bot" PR (#328) that authored channel-outbound.service.ts from scratch; the failure.outcome !== "uncertain" guard that excludes uncertain outcomes from the failure message update was placed in completeClaim on the first version of the file, and recoverExpiredLeases was added in the same commit updating only the intent row, never the message. The pattern already existed in the legacy path — 6aa4306 (#247) had introduced the metadata.error = "send_outcome_unknown" handling in message-handlers.ts months earlier — but the new spine dispatcher did not replicate it. The follow-up 646663a (#344) added realtime fanout to the accepted and failure paths but inserted it only inside the existing !== "uncertain" block, leaving the uncertain branch's behavior byte-identical and so neither caused nor fixed the defect; 811a5cd (#351) merely routed the inbox onto the spine, exposing the pre-existing gap to user traffic.

Activity

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions