Skip to content

Commit c3849ee

Browse files
improvement(chat): address cleanup-pass findings on the Chat gate
Effects: the panel's auto-select effect read the copilot chat list while the list query was deliberately skipped, took "empty" for "deleted in another tab", and cleared the user's selection — latching a ref that stopped it ever being restored. Guarded on the same condition as the handoff listener. Memo: `/w` filtered workflows through a useMemo whose array dependency was a fresh `[]` on every render while the query had no data — the exact window the page exists for — so it memoized nothing and re-fired the redirect effect. Keyed on the workflow id instead. Same unstable-default problem on the sidebar's chat list, where it invalidated five downstream memos; given a stable empty constant. Callback: `handleCreateWorkflow` listed the whole mutation object in its deps, which TanStack recreates every render. Harmless until this branch wired it into the top nav, where it defeated `memo(SidebarNavItem)`. React Query: Recently Deleted still fetched archived chats unconditionally and offered restores into routes that now 404. Also surfaces an error state on `/w` — it is the landing route now, so a failed list fetch would otherwise spin forever behind a log line — fixes a spinner using a token undefined in dark mode, and trims comments that restated code. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012ErcRgvi7VQBeKDQ3MBMha
1 parent bf7c377 commit c3849ee

9 files changed

Lines changed: 67 additions & 40 deletions

File tree

apps/sim/app/workspace/[workspaceId]/page.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,8 @@ import { redirect } from 'next/navigation'
22
import { isChatEnabled } from '@/lib/core/config/env-flags'
33

44
/**
5-
* Resolves the workspace landing route. With Chat enabled that is the chat
6-
* composer; otherwise `/w`, which selects the first workflow from the workflow
7-
* list the layout already prefetched.
5+
* Resolves the workspace landing route: the chat composer, or `/w`, which
6+
* selects the first workflow from the list the layout already prefetched.
87
*
98
* Deliberately does no work of its own. Resolving the workflow here would mean
109
* a session lookup, an access check, and a query before anything renders — and

apps/sim/app/workspace/[workspaceId]/settings/components/recently-deleted/recently-deleted.tsx

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import { useParams, useRouter } from 'next/navigation'
99
import { useQueryStates } from 'nuqs'
1010
import { canMutateWorkspaceSettingsSection } from '@/components/settings/navigation'
1111
import type { ServedFolderResourceType } from '@/lib/api/contracts/folders'
12+
import { isChatEnabled } from '@/lib/core/config/env-flags'
1213
import { type ColumnOption, SortDropdown } from '@/app/workspace/[workspaceId]/components'
1314
import { RESOURCE_REGISTRY } from '@/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry'
1415
import type { MothershipResourceType } from '@/app/workspace/[workspaceId]/home/types'
@@ -226,7 +227,12 @@ export function RecentlyDeleted() {
226227
const tableFoldersQuery = useFolders(workspaceId, { scope: 'archived', resourceType: 'table' })
227228
const filesQuery = useWorkspaceFiles(workspaceId, 'archived')
228229
const workspaceFoldersQuery = useWorkspaceFileFolders(workspaceId, 'archived')
229-
const chatsQuery = useMothershipChats(workspaceId, { scope: 'archived' })
230+
// Restoring a chat navigates to a route that 404s with Chat off, and this
231+
// query's loading/error state feeds the whole panel's.
232+
const chatsQuery = useMothershipChats(workspaceId, {
233+
scope: 'archived',
234+
enabled: isChatEnabled,
235+
})
230236

231237
const restoreWorkflow = useRestoreWorkflow()
232238
const restoreFolder = useRestoreFolder()

apps/sim/app/workspace/[workspaceId]/w/[workflowId]/components/panel/panel.tsx

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -294,7 +294,10 @@ export const Panel = memo(function Panel() {
294294
// chat was deleted in another tab).
295295
const autoSelectAttemptedForRef = useRef<Set<string>>(new Set())
296296
useEffect(() => {
297-
if (!activeWorkflowId) return
297+
// The list query is skipped when the tab is unavailable, so an empty list
298+
// there means "not fetched", not "deleted elsewhere" — clearing on it would
299+
// discard the selection and latch the ref against ever restoring it.
300+
if (!activeWorkflowId || !isCopilotTabAvailable) return
298301

299302
if (copilotChatId && !copilotChatList.find((c) => c.id === copilotChatId)) {
300303
setCopilotChatId(undefined)
@@ -306,7 +309,7 @@ export const Panel = memo(function Panel() {
306309
if (copilotChatList.length === 0) return
307310
autoSelectAttemptedForRef.current.add(activeWorkflowId)
308311
setCopilotChatId(copilotChatList[0].id)
309-
}, [copilotChatList, copilotChatId, activeWorkflowId, setCopilotChatId])
312+
}, [copilotChatList, copilotChatId, activeWorkflowId, isCopilotTabAvailable, setCopilotChatId])
310313

311314
useEffect(() => {
312315
posthogRef.current = posthog

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/hooks/use-workflow-operations.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,14 +23,18 @@ export function useWorkflowOperations({ workspaceId }: UseWorkflowOperationsProp
2323
[workflows, workspaceId]
2424
)
2525

26+
// `mutate` is stable; the mutation object it hangs off is a new literal every
27+
// render, so depending on the object would leave this callback unmemoized.
28+
const createWorkflowMutate = createWorkflowMutation.mutate
29+
2630
const handleCreateWorkflow = useCallback((): Promise<string | null> => {
2731
const { clearDiff } = useWorkflowDiffStore.getState()
2832
clearDiff()
2933

3034
const name = generateCreativeWorkflowName()
3135
const id = generateId()
3236

33-
createWorkflowMutation.mutate({
37+
createWorkflowMutate({
3438
workspaceId,
3539
name,
3640
id,
@@ -39,7 +43,7 @@ export function useWorkflowOperations({ workspaceId }: UseWorkflowOperationsProp
3943
useWorkflowRegistry.getState().markWorkflowCreating(id)
4044
router.push(`/workspace/${workspaceId}/w/${id}`)
4145
return Promise.resolve(id)
42-
}, [createWorkflowMutation, workspaceId, router])
46+
}, [createWorkflowMutate, workspaceId, router])
4347

4448
return {
4549
workflows,

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/sidebar.tsx

Lines changed: 19 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ import { useCustomBlockOverlayVersion } from '@/blocks/custom/client-overlay'
9292
import { useWorkspaceCredentials } from '@/hooks/queries/credentials'
9393
import { useFolderMap, useFolders } from '@/hooks/queries/folders'
9494
import { useKnowledgeBasesQuery } from '@/hooks/queries/kb/knowledge'
95+
import type { MothershipChatMetadata } from '@/hooks/queries/mothership-chats'
9596
import {
9697
useDeleteMothershipChat,
9798
useDeleteMothershipChats,
@@ -117,6 +118,13 @@ import { useSidebarStore } from '@/stores/sidebar/store'
117118

118119
const logger = createLogger('Sidebar')
119120

121+
/**
122+
* Stable identity for the chat list's "no data" case. With Chat disabled the
123+
* query never runs, so a `= []` default would mint a new array every render and
124+
* invalidate every memo downstream of it.
125+
*/
126+
const EMPTY_CHATS: MothershipChatMetadata[] = []
127+
120128
const SLACK_COMMUNITY_URL =
121129
'https://join.slack.com/t/sim-ott9864/shared_invite/zt-43lp8tc5v-0qrrqHGBKUsvQlpoouH~TA'
122130

@@ -733,8 +741,6 @@ export const Sidebar = memo(function Sidebar({
733741
const topNavItems = useMemo(
734742
() =>
735743
[
736-
// Same slot either way: the primary "start something new" action. With
737-
// Chat off that is a workflow, since there is no composer to open.
738744
{
739745
id: 'home',
740746
label: isChatEnabled ? 'New chat' : 'New workflow',
@@ -824,24 +830,23 @@ export const Sidebar = memo(function Sidebar({
824830
[navigateToSettings, getSettingsHref, setSidebarWidth]
825831
)
826832

827-
const { data: fetchedChats = [], isLoading: chatsLoading } = useMothershipChats(workspaceId, {
828-
enabled: isChatEnabled,
829-
})
833+
const { data: fetchedChats = EMPTY_CHATS, isLoading: chatsLoading } = useMothershipChats(
834+
workspaceId,
835+
{ enabled: isChatEnabled }
836+
)
830837

831838
useMothershipChatEvents(workspaceId)
832839

833840
/**
834-
* Empty when Chat is disabled, which also drops the command palette's Chats
835-
* group — `SearchGroups` renders nothing for an empty list.
841+
* Stays empty when Chat is disabled, which also drops the command palette's
842+
* Chats group — `SearchGroups` renders nothing for an empty list.
836843
*/
837844
const chats = useMemo(
838845
() =>
839-
isChatEnabled && fetchedChats
840-
? fetchedChats.map((t) => ({
841-
...t,
842-
href: `/workspace/${workspaceId}/chat/${t.id}`,
843-
}))
844-
: [],
846+
fetchedChats.map((t) => ({
847+
...t,
848+
href: `/workspace/${workspaceId}/chat/${t.id}`,
849+
})),
845850
[fetchedChats, workspaceId]
846851
)
847852

@@ -1406,7 +1411,7 @@ export const Sidebar = memo(function Sidebar({
14061411
>
14071412
{chatsLoading ? (
14081413
<DropdownMenuItem disabled>
1409-
<Loader className='h-[14px] w-[14px]' animate />
1414+
<Loader className='size-[14px]' animate />
14101415
Loading...
14111416
</DropdownMenuItem>
14121417
) : chats.length === 0 ? (

apps/sim/app/workspace/[workspaceId]/w/page.tsx

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use client'
22

3-
import { useEffect, useMemo } from 'react'
3+
import { useEffect } from 'react'
44
import { Chip } from '@sim/emcn'
55
import { createLogger } from '@sim/logger'
66
import { useParams, useRouter } from 'next/navigation'
@@ -17,7 +17,7 @@ function Spinner() {
1717
className='size-[18px] animate-spin rounded-full'
1818
style={{
1919
background:
20-
'conic-gradient(from 0deg, hsl(var(--muted-foreground)) 0deg 120deg, transparent 120deg 180deg, hsl(var(--muted-foreground)) 180deg 300deg, transparent 300deg 360deg)',
20+
'conic-gradient(from 0deg, var(--text-icon) 0deg 120deg, transparent 120deg 180deg, var(--text-icon) 180deg 300deg, transparent 300deg 360deg)',
2121
mask: 'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
2222
WebkitMask:
2323
'radial-gradient(farthest-side, transparent calc(100% - 1.5px), black calc(100% - 1.5px))',
@@ -34,10 +34,10 @@ export default function WorkflowsPage() {
3434
const { data: workflows = [], isLoading, isError, isPlaceholderData } = useWorkflows(workspaceId)
3535
const { handleCreateWorkflow, isCreatingWorkflow } = useWorkflowOperations({ workspaceId })
3636

37-
const workspaceWorkflows = useMemo(
38-
() => workflows.filter((w) => w.workspaceId === workspaceId),
39-
[workflows, workspaceId]
40-
)
37+
// An id rather than the filtered array: `data` defaults to a fresh `[]` while
38+
// the query has no data, so an array dependency would re-fire this on every
39+
// render — exactly during the load this page exists to cover.
40+
const firstWorkflowId = workflows.find((w) => w.workspaceId === workspaceId)?.id
4141
const isResolving = isLoading || isPlaceholderData
4242

4343
useEffect(() => {
@@ -48,30 +48,42 @@ export default function WorkflowsPage() {
4848
return
4949
}
5050

51-
if (workspaceWorkflows.length > 0) {
52-
router.replace(`/workspace/${workspaceId}/w/${workspaceWorkflows[0].id}`)
51+
if (firstWorkflowId) {
52+
router.replace(`/workspace/${workspaceId}/w/${firstWorkflowId}`)
5353
}
54-
}, [isResolving, isError, workspaceWorkflows, workspaceId, router])
54+
}, [isResolving, isError, firstWorkflowId, workspaceId, router])
5555

5656
/**
5757
* A workspace can legitimately reach zero workflows — deleting the last one,
5858
* archiving them all, or creating a workspace with `skipDefaultWorkflow`. This
5959
* is the terminal state for those paths now that the chat composer is no
6060
* longer a landing option, so it has to offer a way out rather than spin.
6161
*/
62-
const isEmpty = !isResolving && !isError && workspaceWorkflows.length === 0
62+
const isEmpty = !isResolving && !isError && !firstWorkflowId
6363

6464
return (
6565
<div className='flex h-full w-full flex-col overflow-hidden bg-[var(--bg)]'>
6666
<div className='relative h-full w-full flex-1 bg-[var(--bg)]'>
6767
<div className='workflow-container flex h-full items-center justify-center bg-[var(--bg)]'>
68-
{isEmpty ? (
68+
{isError ? (
69+
// This is the landing route now, so a failed list fetch would
70+
// otherwise spin forever with nothing but a log line.
71+
<div className='flex flex-col items-center gap-3 text-center text-[var(--text-secondary)]'>
72+
<div>
73+
<p className='font-medium text-small'>Couldn't load workflows</p>
74+
<p className='mt-1 text-caption'>Check your connection and try again.</p>
75+
</div>
76+
<Chip variant='primary' onClick={() => router.refresh()}>
77+
Retry
78+
</Chip>
79+
</div>
80+
) : isEmpty ? (
6981
<div className='flex flex-col items-center gap-3 text-center text-[var(--text-secondary)]'>
7082
<div>
7183
<p className='font-medium text-small'>No workflows yet</p>
7284
<p className='mt-1 text-caption'>Create one to start building.</p>
7385
</div>
74-
<Chip onClick={handleCreateWorkflow} disabled={isCreatingWorkflow}>
86+
<Chip variant='primary' onClick={handleCreateWorkflow} disabled={isCreatingWorkflow}>
7587
{isCreatingWorkflow ? 'Creating…' : 'Create workflow'}
7688
</Chip>
7789
</div>

apps/sim/lib/core/config/env-flags.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,10 +67,9 @@ export const isCopilotBillingProtocolRequired = isTruthy(env.COPILOT_BILLING_PRO
6767
* This governs presentation only. Whether Chat can actually reach the mothership
6868
* is a separate question answered by `COPILOT_API_KEY`, which gates the paths
6969
* that need it (the Sim Chat block, prompt-job claims, inbox execution). Keeping
70-
* them separate is what lets this be a single variable: the key is a secret and
71-
* could never be read in the browser, but `NEXT_PUBLIC_CHAT_DISABLED` is not, so
72-
* `getEnv` resolves the same value from `process.env` on the server and
73-
* `window.__ENV` on the client — no twin to keep in sync.
70+
* them separate is what lets this be a single variable: the secret key could
71+
* never be read in the browser, but `NEXT_PUBLIC_CHAT_DISABLED` can — no twin to
72+
* keep in sync.
7473
*
7574
* Read at module scope or inline during render only. Resolving it through
7675
* `useState`/`useEffect` would render chat surfaces before removing them.

apps/sim/lib/core/config/env.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -605,7 +605,7 @@ export const env = createEnv({
605605
NEXT_PUBLIC_DISABLE_INVITATIONS: z.boolean().optional(), // Disable workspace invitations globally (for self-hosted deployments)
606606
NEXT_PUBLIC_DISABLE_PUBLIC_API: z.boolean().optional(), // Disable public API access UI toggle globally
607607
NEXT_PUBLIC_INBOX_ENABLED: z.boolean().optional(), // Enable inbox (Sim Mailer) on self-hosted
608-
NEXT_PUBLIC_CHAT_DISABLED: z.boolean().optional(), // Hide the Chat module. Not a secret, so it is read via getEnv() on both server and client
608+
NEXT_PUBLIC_CHAT_DISABLED: z.boolean().optional(), // Hide the Chat module (Chat is shown when unset)
609609
NEXT_PUBLIC_SANDBOXES_ENABLED: z.boolean().optional(), // Enable custom sandboxes on self-hosted
610610
NEXT_PUBLIC_EMAIL_PASSWORD_SIGNUP_ENABLED: z.boolean().optional().default(true), // Control visibility of email/password login forms
611611
NEXT_PUBLIC_TURNSTILE_SITE_KEY: z.string().min(1).optional(), // Cloudflare Turnstile site key for captcha widget

scripts/setup/modes/k8s.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -282,7 +282,6 @@ interface ReleaseValues {
282282
postgresql?: { auth?: { password?: string } }
283283
}
284284

285-
/** Values of the installed release, or `null` when there is no release yet. */
286285
function existingReleaseValues(context: string): ReleaseValues | null {
287286
const scope = ['--kube-context', context, '-n', NAMESPACE]
288287
const status = spawnSync('helm', ['status', RELEASE, ...scope], { stdio: 'ignore' })

0 commit comments

Comments
 (0)