-
Notifications
You must be signed in to change notification settings - Fork 3.6k
feat(copilot): preserve mothership chat when opening workflow #4521
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: staging
Are you sure you want to change the base?
Changes from all commits
528bcfb
6be5374
e8a692e
bc431ff
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,7 +5,7 @@ import { createLogger } from '@sim/logger' | |
| import { toError } from '@sim/utils/errors' | ||
| import { useQueryClient } from '@tanstack/react-query' | ||
| import { History, Plus } from 'lucide-react' | ||
| import { useParams, useRouter } from 'next/navigation' | ||
| import { useParams, useRouter, useSearchParams } from 'next/navigation' | ||
| import { usePostHog } from 'posthog-js/react' | ||
| import { useShallow } from 'zustand/react/shallow' | ||
| import { | ||
|
|
@@ -46,7 +46,11 @@ import { captureEvent } from '@/lib/posthog/client' | |
| import { generateWorkflowJson } from '@/lib/workflows/operations/import-export' | ||
| import { ConversationListItem } from '@/app/workspace/[workspaceId]/components' | ||
| import { MothershipChat } from '@/app/workspace/[workspaceId]/home/components' | ||
| import { getWorkflowCopilotUseChatOptions, useChat } from '@/app/workspace/[workspaceId]/home/hooks' | ||
| import { | ||
| getMothershipUseChatOptions, | ||
| getWorkflowCopilotUseChatOptions, | ||
| useChat, | ||
| } from '@/app/workspace/[workspaceId]/home/hooks' | ||
| import type { FileAttachmentForApi } from '@/app/workspace/[workspaceId]/home/types' | ||
| import { useRegisterGlobalCommands } from '@/app/workspace/[workspaceId]/providers/global-commands-provider' | ||
| import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/providers/workspace-permissions-provider' | ||
|
|
@@ -118,7 +122,9 @@ interface PanelProps { | |
| export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: PanelProps = {}) { | ||
| const router = useRouter() | ||
| const params = useParams() | ||
| const searchParams = useSearchParams() | ||
| const workspaceId = propWorkspaceId ?? (params.workspaceId as string) | ||
| const urlChatIdParam = searchParams?.get('chatId') ?? null | ||
|
|
||
| const posthog = usePostHog() | ||
| const posthogRef = useRef(posthog) | ||
|
|
@@ -250,11 +256,23 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
| ) | ||
| const [isCopilotHistoryOpen, setIsCopilotHistoryOpen] = useState(false) | ||
|
|
||
| const copilotChatTitle = useMemo( | ||
| () => | ||
| copilotChatId ? (copilotChatList.find((c) => c.id === copilotChatId)?.title ?? null) : null, | ||
| const selectedCopilotChat = useMemo( | ||
| () => (copilotChatId ? (copilotChatList.find((c) => c.id === copilotChatId) ?? null) : null), | ||
| [copilotChatId, copilotChatList] | ||
| ) | ||
| const copilotChatTitle = selectedCopilotChat?.title ?? null | ||
|
|
||
| /** | ||
| * A selected chat is "foreign" to this workflow when it was started in | ||
| * Mothership (`type === 'mothership'`) but ended up referencing this | ||
| * workflow via `resources`. We keep the conversation continuable by | ||
| * routing sends through the Mothership branch — i.e. the request goes | ||
| * out without `workflowId`, so the server uses the broader Mothership | ||
| * agent surface that originally produced the chat. The trade-off is | ||
| * that resources spawned during continuation only show up in the | ||
| * Mothership view; this panel shows the conversation only. | ||
| */ | ||
| const isMothershipChat = selectedCopilotChat?.type === 'mothership' | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Mothership chat read-only mode never applied to UIHigh Severity
Additional Locations (1)Reviewed by Cursor Bugbot for commit 6be5374. Configure here.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Stale flag on this thread. The "input footer is hidden / read-only" wording was from an earlier iteration — this commit (and the PR description) deliberately switched to continuation: the panel swaps |
||
|
|
||
| const queryClient = useQueryClient() | ||
| const loadCopilotChats = useCallback(() => { | ||
|
|
@@ -264,8 +282,14 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
|
|
||
| // Auto-select most recent on first list arrival per workflow, and drop a | ||
| // selection that no longer matches anything in the current list (e.g. the | ||
| // chat was deleted in another tab). | ||
| // chat was deleted in another tab). When a `?chatId=` param is present in | ||
| // the URL (e.g. after clicking "Open Workflow" from a Mothership task), | ||
| // prefer that chat over the most recent so the original conversation is | ||
| // shown right away. The URL param is honored once per distinct value so | ||
| // returning to a workflow with a fresh `?chatId=` re-applies it instead of | ||
| // being shadowed by the once-per-workflow auto-select guard. | ||
| const autoSelectAttemptedForRef = useRef<Set<string>>(new Set()) | ||
| const consumedUrlChatIdRef = useRef<string | null>(null) | ||
| useEffect(() => { | ||
| if (!activeWorkflowId) return | ||
|
|
||
|
|
@@ -274,12 +298,23 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
| return | ||
| } | ||
|
|
||
| if ( | ||
| urlChatIdParam && | ||
| consumedUrlChatIdRef.current !== urlChatIdParam && | ||
| copilotChatList.find((c) => c.id === urlChatIdParam) | ||
| ) { | ||
| consumedUrlChatIdRef.current = urlChatIdParam | ||
| autoSelectAttemptedForRef.current.add(activeWorkflowId) | ||
| setCopilotChatId(urlChatIdParam) | ||
| return | ||
| } | ||
|
|
||
| if (copilotChatId) return | ||
| if (autoSelectAttemptedForRef.current.has(activeWorkflowId)) return | ||
| if (copilotChatList.length === 0) return | ||
| autoSelectAttemptedForRef.current.add(activeWorkflowId) | ||
| setCopilotChatId(copilotChatList[0].id) | ||
| }, [copilotChatList, copilotChatId, activeWorkflowId, setCopilotChatId]) | ||
| }, [copilotChatList, copilotChatId, activeWorkflowId, setCopilotChatId, urlChatIdParam]) | ||
|
Comment on lines
291
to
+317
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Addressed in |
||
|
|
||
| useEffect(() => { | ||
| posthogRef.current = posthog | ||
|
|
@@ -335,6 +370,40 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
| [activeWorkflowId] | ||
| ) | ||
|
|
||
| const handleCopilotRequestStarted = useCallback( | ||
| ({ requestId, userMessageId }: { requestId: string; userMessageId: string }) => { | ||
| captureEvent(posthogRef.current, 'task_request_started', { | ||
| workspace_id: workspaceId, | ||
| view: 'copilot', | ||
| request_id: requestId, | ||
| user_message_id: userMessageId, | ||
| }) | ||
| }, | ||
| [workspaceId] | ||
| ) | ||
|
|
||
| const copilotChatOptions = useMemo( | ||
| () => | ||
| isMothershipChat | ||
| ? getMothershipUseChatOptions({ | ||
| onStreamEnd: loadCopilotChats, | ||
| onRequestStarted: handleCopilotRequestStarted, | ||
| }) | ||
| : getWorkflowCopilotUseChatOptions({ | ||
| workflowId: activeWorkflowId || undefined, | ||
| onTitleUpdate: loadCopilotChats, | ||
| onToolResult: handleCopilotToolResult, | ||
| onRequestStarted: handleCopilotRequestStarted, | ||
| }), | ||
| [ | ||
| isMothershipChat, | ||
| activeWorkflowId, | ||
| loadCopilotChats, | ||
| handleCopilotToolResult, | ||
| handleCopilotRequestStarted, | ||
| ] | ||
| ) | ||
|
|
||
| const { | ||
| messages: copilotMessages, | ||
| isSending: copilotIsSending, | ||
|
|
@@ -347,23 +416,7 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
| sendNow: copilotSendNow, | ||
| editQueuedMessage: copilotEditQueuedMessage, | ||
| getCurrentRequestId: getCopilotCurrentRequestId, | ||
| } = useChat( | ||
| workspaceId, | ||
| copilotChatId, | ||
| getWorkflowCopilotUseChatOptions({ | ||
| workflowId: activeWorkflowId || undefined, | ||
| onTitleUpdate: loadCopilotChats, | ||
| onToolResult: handleCopilotToolResult, | ||
| onRequestStarted: ({ requestId, userMessageId }) => { | ||
| captureEvent(posthogRef.current, 'task_request_started', { | ||
| workspace_id: workspaceId, | ||
| view: 'copilot', | ||
| request_id: requestId, | ||
| user_message_id: userMessageId, | ||
| }) | ||
| }, | ||
| }) | ||
| ) | ||
| } = useChat(workspaceId, copilotChatId, copilotChatOptions) | ||
|
|
||
| const handleCopilotNewChat = useCallback(() => { | ||
| if (!activeWorkflowId || !workspaceId) return | ||
|
|
@@ -444,6 +497,20 @@ export const Panel = memo(function Panel({ workspaceId: propWorkspaceId }: Panel | |
| setHasHydrated(true) | ||
| }, [setHasHydrated]) | ||
|
|
||
| /** | ||
| * If the workflow page was opened with `?chatId=`, surface the copilot | ||
| * tab so the linked conversation is visible without an extra click. | ||
| * Re-applies whenever the param changes so returning to a workflow with | ||
| * a fresh `?chatId=` switches the tab again. | ||
| */ | ||
| const handledTabSwitchForChatIdRef = useRef<string | null>(null) | ||
| useEffect(() => { | ||
| if (!urlChatIdParam) return | ||
| if (handledTabSwitchForChatIdRef.current === urlChatIdParam) return | ||
| handledTabSwitchForChatIdRef.current = urlChatIdParam | ||
| setActiveTab('copilot') | ||
| }, [urlChatIdParam, setActiveTab]) | ||
|
|
||
| useEffect(() => { | ||
| const handler = (e: Event) => { | ||
| const message = (e as CustomEvent<{ message: string }>).detail?.message | ||
|
|
||


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Open Workflow navigates same tab instead of new tab
High Severity
handleOpenWorkflowwas changed fromwindow.open(url, '_blank')(opens a new tab) torouter.push(url)(same-tab navigation). This means clicking "Open Workflow" from a Mothership task now navigates away from the Mothership view, causing the user to lose their in-progress conversation context. The PR test plan explicitly expects "a new tab opens," confirming this is unintended. The originalwindow.opencall just needs the?chatId=query string appended while keeping'_blank'.Reviewed by Cursor Bugbot for commit bc431ff. Configure here.