Visitar URL original
refactor(mothership): one re-queue policy and one send payload, and keep a Stop-failed send waiting for the user by waleedlatif1 · Pull Request #8740 · simstudioai/sim · GitHub
Skip to content

refactor(mothership): one re-queue policy and one send payload, and keep a Stop-failed send waiting for the user - #8740

Merged
waleedlatif1 merged 2 commits into
stagingfrom
refactor/mothership-requeue-policy
Oct 7, 2026
Merged

refactor(mothership): one re-queue policy and one send payload, and keep a Stop-failed send waiting for the user#8740
waleedlatif1 merged 2 commits into
stagingfrom
refactor/mothership-requeue-policy

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator

Summary

Second step of the chat send-queue consolidation (after #8738): one re-queue policy, one payload type, and the G3 fix.

  • One withdrawal reason. A send handed back to its caller now carries reason: 'withdrawn' | 'offline' | 'unreachable' | 'busy' | 'stop-failed', instead of five mutually exclusive booleans (unreachable, heldUntilOnline, busy, held, and the implied plain withdrawal).
  • One re-queue policy. A pure requeuedFields(reason, previousAttempts, chatlessSurface) in the new send-queue-policy.ts decides when, and on which surface, a re-queued message goes out. Both re-queue sites (sendMessage after a failed direct send, and the queue dispatch's restore) use it. A send that failed outright, with no withdrawal, is failed and waits for the user.
  • G3 fixed. The restore spread the dispatched entry back with whatever hold, retry and surface fields an earlier outcome had set. A message held for the network that the user then sent by hand over a running turn, whose Stop did not settle, came back waiting for the user but still marked heldUntilOnline. The browser coming online then released and sent it. The restore now strips those fields (withoutRequeueFields) before applying the policy for this outcome.
  • One payload type. SendPayload (content, attachments, contexts, mode, search settings) replaces the positional parameters of createQueuedMessage and sendMothershipMessage and about ten hand-spread copies. sendPayload() is the one place that drops the fields a send doesn't set. QueuedMessage extends it. Callers of sendMothershipMessage outside chat (log details, terminal, workflow block, search modal, console store) pass an object now.
  • Comment fix: the head re-queue comment now notes the one exception: a held send adopted from a dead mount while the POST was out can be older.

Type of Change

  • Bug fix
  • Improvement (refactor)

Testing

  • keeps a Send-now whose Stop failed waiting for the user when the browser comes online (DOM) fails on staging and passes here.
  • send-queue-policy.test.ts unit-tests the policy for each reason, the field stripping, and the payload normalizer.
  • The existing chat suites pass unchanged. The full gate and integration tests ran on the CI runner.

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)

@waleedlatif1
refactor(mothership): one re-queue policy for a withdrawn send
197a9ce 
A withdrawn send's outcome is one reason (withdrawn, offline, unreachable,
busy, stop-failed) instead of five exclusive booleans, and a pure
requeuedFields(reason, previousAttempts, chatlessSurface) is the policy both
re-queue sites apply. The restore strips the hold, retry and surface fields
an earlier outcome left before applying it, so a stale heldUntilOnline can no
longer let the browser coming online send a message waiting for the user.
@waleedlatif1
refactor(mothership): pass a chat send as one SendPayload
2525c4b 
A send's content, attachments, contexts, mode and search settings travel as
one SendPayload instead of positional parameters (createQueuedMessage,
sendMothershipMessage) and about ten hand-spread copies; sendPayload() is the
one place that drops the fields a send does not set.
@vercel

vercel Bot commented Oct 7, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated
docs Skipped Skipped Oct 7, 2026 9:38am UTC

Request Review

@waleedlatif1

waleedlatif1 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

waleedlatif1 commented Oct 7, 2026

Copy link
Copy Markdown
Collaborator Author

@cubic-dev-ai review this PR

@cubic-dev-ai

cubic-dev-ai Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

@cubic-dev-ai review this PR

@waleedlatif1 I have started the AI code review. It will take a few minutes to complete.

cubic-dev-ai[bot]
cubic-dev-ai Bot reviewed Oct 7, 2026
View reviewed changes

@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.

No issues found across 12 files

Confidence score: 5/5

  • Automated review surfaced no issues in the provided summaries.
  • No files require special attention.

Turn on auto-fix | Re-trigger cubic

@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors message send handling and queue retry logic.

This PR appears safe to merge; no actionable issues were found.

What we checked:

  • Online sends after failed Stop: withoutRequeueFields removes the old online hold. The new policy sets only retryRequired, so coming online cannot release this message.
  • Queued message lost on remount: Unmount cleanup marks the remaining queue for the same surface. Its next mount adopts those messages, including messages waiting for the user.

Summary

This PR gives chat sends one payload type and one policy for returning messages to the queue. It also removes stale queue fields so a failed Stop leaves the message waiting for the user.

  • Both queue restore paths use requeuedFields.
  • Chat actions now pass a SendPayload object.
  • The new regression test checks that coming online does not release a message whose Stop failed.
  • No actionable issues found. Tests were inspected, not run.
Diagram 
%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Send returns to caller] --> B[Remove earlier hold and retry fields]
  B --> C{Current reason}
  C -->|offline| D[Wait for online or user]
  C -->|unreachable or busy| E[Retry after delay]
  C -->|stop-failed or failed| F[Wait for user]
  C -->|withdrawn| G[Resume queue or hand off]
Loading

Reviews (1) · Last reviewed commit: "refactor(mothership): pass a chat send a..." · Reviewed by Greptile

@waleedlatif1
waleedlatif1 merged commit f83d977 into staging Oct 7, 2026
33 checks passed
@waleedlatif1 waleedlatif1 mentioned this pull request Oct 7, 2026
Merged
6 tasks done
@waleedlatif1
waleedlatif1 deleted the refactor/mothership-requeue-policy branch October 7, 2026 16:32
@waleedlatif1 waleedlatif1 mentioned this pull request Oct 7, 2026
Merged

This branch was previously deployed

1 inactive deployment
Preview — 2525c4b1 Deployed Oct 7, 2026 by vercel[bot]
View deployment
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@cubic-dev-ai cubic-dev-ai[bot] cubic-dev-ai[bot] left review comments

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@waleedlatif1