Skip to content

fix(mothership): cancel desktop tools a signed-out turn starts late, guard the shown-once license key - #8757

Merged
waleedlatif1 merged 4 commits into
stagingfrom
fix/mothership-chat-effort-and-guards
Oct 7, 2026
Merged

waleedlatif1 merged 4 commits into
stagingfrom
fix/mothership-chat-effort-and-guards

Conversation

@waleedlatif1

@waleedlatif1 waleedlatif1 commented Oct 7, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

  • Desktop tool leases are bound to the session the chat surface mounted in (desktopToolSession(), held once per useChat mount). Sign-out (stopAllDesktopTools) aborts that session and starts a new one. A send still pending at sign-out, or a reconnect of an old stream, delivers its tool events to a surface mounted before the stop, so each of them gets an already-aborted lease instead of a fresh controller. Signing out leaves or reloads every chat surface, so surfaces mounted after sign-in run normally, with no reset hook needed.
  • No per-stream tombstone after a single turn's Stop: a withdrawn first send retries with the same message id (= stream id), so a tombstone would pre-abort the retry's tools. Stop already cancels that turn's stream reader.
  • Mothership admin Licenses tab: the shown-once generated license key counts as unsaved, so switching tabs or environments asks first. Confirming the discard drops the key.
  • The new-chat effort contract (optional effort in the chat response) and the effort pick's lifetime across the first-send composer swap already landed in fix(mothership): close a pre-aborted SSE stream, keep the new-chat effort across a failed first send #8754, with regression tests.

Type of Change

  • Bug fix

Testing

  • desktop-tool-lifetimes.test.ts: leases from a surface mounted before sign-out, including on a stream it reads later, are aborted with the sign-out reason (fails with the session guard reverted), and a surface mounted after sign-out gets live leases. stores/index.test.ts was updated to the new interface.
  • mothership.test.tsx (new): after generating a key, leaving is refused until confirmed, and confirming drops the key. Both tests fail on the pre-fix code.
  • bun run lint, bun run type-check, bun run check:audits, docs-manifest:check, block registry check, and the home/settings/stores vitest suites. Root bun run test passes except two CPU-heavy tests that time out under local load (archive.test.ts, remark-plain-text.test.ts); this PR doesn't touch them.

Checklist

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

@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:52pm UTC

Request Review

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

All reported issues were addressed across 6 files

Reply with feedback, questions, or to request a fix.

Turn on auto-fix | Re-trigger cubic

Comment thread apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Outdated
@greptile-apps

greptile-apps Bot commented Oct 7, 2026 •

Copy link
Copy Markdown
Contributor

RetriggerConfidence Score: 5/5

[Medium risk] Refactors desktop tool lifecycle management for session binding.

This PR appears safe to merge; no actionable issues remain.

What we checked:

  • Late tools stay canceled: The chat keeps its original session handle. Sign-out aborts that session, so later leases return an already-aborted signal that desktop executors check before starting work.

Summary

This PR binds desktop tools to the session captured when useChat mounts. Tools delivered after sign-out receive an already-aborted signal, including events from a late response or reconnect.

  • Both previous, unnumbered findings are addressed: session capture no longer happens after the request, and the license tests no longer assert mock calls.
  • Adds tests for leaving the Licenses tab and discarding the shown-once key when switching environments.
  • No new actionable issues were found. Tests were not run during this review.
Diagram
sequenceDiagram
  participant Chat as Chat surface
  participant Session as Desktop tool session
  participant Cleanup as Sign-out
  participant Tool as Desktop tool
  Chat->>Session: Capture session on mount
  Cleanup->>Session: Abort and replace session
  Chat->>Session: Lease tool from late stream event
  Session-->>Tool: Already-aborted signal
  Tool->>Tool: Skip desktop work
Loading

Reviews (4) · Last reviewed commit: "test(mothership): mock Chip for the lice..." · Reviewed by Greptile

Comment thread apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts Outdated
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

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 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 6 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

@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1

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 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 6 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

…guard the shown-once license key

- A turn's stream now binds to the session it started in. Its tool events can arrive after sign-out stops every desktop tool, and each one then gets an already-aborted lease instead of a fresh controller. A turn started after sign-in runs normally.
- The generated license key counts as an unsaved change, so leaving the Licenses tab asks first, and confirming drops it.
…ounted in

A send or reconnect still in flight at sign-out reaches the stream reader after the stop, so a per-reader capture took the new session. The surface now takes its session once at mount; signing out leaves or reloads every chat surface.
…8761

#8761 already guards the shown-once license key (generatedKey in isDirty, cleared on discard), so this branch keeps staging's settings page and its tests now assert that guard. The page now renders Chip, which the shared emcn mock does not provide.
@waleedlatif1

Copy link
Copy Markdown
Collaborator Author

@greptile

@waleedlatif1
waleedlatif1 force-pushed the fix/mothership-chat-effort-and-guards branch from 210ff35 to 5ac72c9 Compare October 7, 2026 21:52
@waleedlatif1

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 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 5 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

@waleedlatif1
waleedlatif1 enabled auto-merge (squash) October 7, 2026 21:57
@waleedlatif1
waleedlatif1 merged commit b7fa794 into staging Oct 7, 2026
38 checks passed
@waleedlatif1
waleedlatif1 deleted the fix/mothership-chat-effort-and-guards branch October 7, 2026 22:55

This branch was previously deployed

1 inactive deployment
Preview — 5ac72c90 Deployed Oct 7, 2026 by vercel[bot]
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