Skip to content

Commit 1a1487d

Browse files
waleedlatif1Waleed Latifclaude
authored
fix(search): gate every Sim Search surface on the flag that already refuses it (#7523)
* fix(search): gate every Sim Search surface on the flag that already refuses it Sim Search shipped behind `knowledge-member-access`, and in production that flag is off: the AppConfig feature-flags document has no entry for it, so the server refuses every per-member connect and hides every member-scoped document. The UI never asked. The Search tab, the Search page and its whole connector catalog, the composer's Search and Assistant modes, and the source chips all rendered unconditionally, so an unreleased feature was visible to everyone — and the connect it offered failed with a message that reads like a plan limit rather than a feature that has not shipped. The flag was only ever wired to the actions and the data, never to the surfaces. Wire the surfaces to the same judgement: - one reader, `useMemberAccessAvailable`, replacing five copies of `features?.knowledgeMemberAccess === true`, so no surface can drift - the Search tab appears only where the page it links to is served - the Search page authenticates and 404s where the feature is off, so the route cannot be typed, linked, or bookmarked into - `useMothershipMode` is the single choke point for the composer: Search and Assistant read as Build and refuse to be written where the feature is off, so a stale `?mode=search` link cannot strand a composer whose switcher is not rendered to leave it; the switcher itself is hidden through the `canSearch` prop the composer already had The search page's module-graph baseline is re-recorded for the session read its gate now performs, matching `settings/[section]/page.tsx`. The modules are already in the route's server bundle through the workspace layout, which reads the same session. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JmUGTitFeoQXVf9T3skEa * fix(search): stop asking the model to cite where the chat cannot render one The `<source>` citation contract shipped with Sim Search in #7385: the tag, the inline chip, and the sources strip under a reply all arrived together. The tool that asks for it did not arrive gated. `manage_knowledge_base`'s query result appends the citation instruction unconditionally, and that tool runs in ordinary Build turns — so in a workspace without per-member access the model is still told to emit `<source>` tags and the chat still renders the chips and the sources popover. An unreleased surface, reachable today without touching Search at all. Ask for the citation only where per-member access is on. The gate belongs at the emission rather than at the renderer: a client that merely declined to render the tag would leave the raw `<source>{...}</source>` JSON sitting in the visible reply. Resolved alongside the search it accompanies, so the gate costs no round trip. Also from the review round: - `handleSubmit` re-applies the gate where the mode is consumed. `modeOverride` is passed straight in and never passes through `useMothershipMode`, so the hook alone was not the choke point the last commit claimed. - the `?q=` effect returns early instead of calling a setter the hook would drop, so the gate reads at the call site rather than at a distance. - `setMode` keeps returning its promise, so the `void` at its three call sites still means what it says. - comments: restore the subject the shortened one-liners had lost, drop a duplicated why, and stop the client hook from re-listing the server predicate's ingredients. - drop three hook tests the switcher's own tests already cover through the same adapter, and the container one of them leaked. Kept deliberately, against a reviewer's suggestion to delete them as now unreachable: the `memberAccessAvailable` guards on the member-connector queries. `useWorkspaceMemberConnectors` sets `placeholderData: keepPreviousData`, so a disabled observer can still surface previously cached rows — the render-time coercion to `EMPTY_MEMBER_CONNECTORS` is what stops rows from a flag-on render leaking into a flag-off one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JmUGTitFeoQXVf9T3skEa * fix(search): never fail a knowledge query over a citation decision Whether to ask for a citation is answered by a billing-backed availability lookup, and it sat in the same `Promise.all` as the search itself — so a rejection there discarded a search that had already succeeded and returned "Failed to query knowledge base". A presentation choice could take out the query it decorates. Settle it to "do not cite" instead — the same answer the feature being off gives — and log why, so the lookup failing is visible without being fatal. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014JmUGTitFeoQXVf9T3skEa --------- Co-authored-by: Waleed Latif <waleed@simstudio.ai> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent a5badf0 commit 1a1487d

18 files changed

Lines changed: 366 additions & 59 deletions

File tree

Lines changed: 64 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,64 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { createRoot, type Root } from 'react-dom/client'
6+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const { mockMemberAccessAvailable } = vi.hoisted(() => ({
9+
mockMemberAccessAvailable: vi.fn(() => true),
10+
}))
11+
12+
vi.mock('@/hooks/use-member-access', () => ({
13+
useMemberAccessAvailable: () => mockMemberAccessAvailable(),
14+
}))
15+
16+
import { IntegrationTabsHeader } from '@/app/workspace/[workspaceId]/components/integration-tabs-header/integration-tabs-header'
17+
18+
let root: Root | null = null
19+
let container: HTMLDivElement | null = null
20+
21+
function mount(active: 'integrations' | 'skills' | 'search' = 'integrations') {
22+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
23+
container = document.createElement('div')
24+
document.body.appendChild(container)
25+
root = createRoot(container)
26+
act(() => root?.render(<IntegrationTabsHeader active={active} workspaceId='workspace-1' />))
27+
}
28+
29+
function tabs(): string[] {
30+
return Array.from(container?.querySelectorAll('a') ?? []).map((node) => node.textContent ?? '')
31+
}
32+
33+
beforeEach(() => {
34+
mockMemberAccessAvailable.mockReturnValue(true)
35+
})
36+
37+
afterEach(() => {
38+
if (root) act(() => root?.unmount())
39+
container?.remove()
40+
root = null
41+
container = null
42+
})
43+
44+
describe('IntegrationTabsHeader', () => {
45+
it('links every tab to its page in the routed workspace', () => {
46+
mount()
47+
48+
expect(tabs()).toEqual(['Integrations', 'Skills', 'Search'])
49+
expect(
50+
Array.from(container?.querySelectorAll('a') ?? []).map((node) => node.getAttribute('href'))
51+
).toEqual([
52+
'/workspace/workspace-1/integrations',
53+
'/workspace/workspace-1/skills',
54+
'/workspace/workspace-1/search',
55+
])
56+
})
57+
58+
it('omits Search where per-member access is off, matching the page that 404s', () => {
59+
mockMemberAccessAvailable.mockReturnValue(false)
60+
mount()
61+
62+
expect(tabs()).toEqual(['Integrations', 'Skills'])
63+
})
64+
})

apps/sim/app/workspace/[workspaceId]/components/integration-tabs-header/integration-tabs-header.tsx

Lines changed: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1+
'use client'
2+
13
import type { ReactNode } from 'react'
24
import { ChipLink, cn } from '@sim/emcn'
35
import { HEADER_ACTION_CLUSTER, PAGE_HEADER_BAR } from '@/components/page-header-bar'
6+
import { useMemberAccessAvailable } from '@/hooks/use-member-access'
47

58
interface IntegrationTabsHeaderProps {
69
active: 'integrations' | 'skills' | 'search'
@@ -17,6 +20,11 @@ interface IntegrationTabsHeaderProps {
1720
* because every page owns it equally; its former home made Skills reach across
1821
* into a sibling feature for its own chrome.
1922
*
23+
* Search appears only where per-member access is on, matching the page it links
24+
* to, which 404s otherwise. A client component so the three pages and their
25+
* Suspense fallbacks all read that one judgement from the workspace host
26+
* context rather than each resolving it again on the server.
27+
*
2028
* The `gap-1` is explicit because chips carry no outer margin — the parent owns the
2129
* space between them.
2230
*/
@@ -25,6 +33,8 @@ export function IntegrationTabsHeader({
2533
workspaceId,
2634
rightSlot,
2735
}: IntegrationTabsHeaderProps) {
36+
const memberAccessAvailable = useMemberAccessAvailable()
37+
2838
return (
2939
<div className={cn(PAGE_HEADER_BAR, 'gap-1')}>
3040
<ChipLink href={`/workspace/${workspaceId}/integrations`} active={active === 'integrations'}>
@@ -33,9 +43,11 @@ export function IntegrationTabsHeader({
3343
<ChipLink href={`/workspace/${workspaceId}/skills`} active={active === 'skills'}>
3444
Skills
3545
</ChipLink>
36-
<ChipLink href={`/workspace/${workspaceId}/search`} active={active === 'search'}>
37-
Search
38-
</ChipLink>
46+
{memberAccessAvailable && (
47+
<ChipLink href={`/workspace/${workspaceId}/search`} active={active === 'search'}>
48+
Search
49+
</ChipLink>
50+
)}
3951
{rightSlot && <div className={cn('ml-auto', HEADER_ACTION_CLUSTER)}>{rightSlot}</div>}
4052
</div>
4153
)

apps/sim/app/workspace/[workspaceId]/home/components/knowledge-search-results/knowledge-search-results.tsx

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -25,12 +25,12 @@ import {
2525
searchFilterParsers,
2626
UPDATED_WINDOWS,
2727
} from '@/app/workspace/[workspaceId]/home/search-params'
28-
import { useWorkspaceHostContext } from '@/app/workspace/[workspaceId]/providers/workspace-host-provider'
2928
import {
3029
useWorkspaceMemberConnectors,
3130
type WorkspaceMemberConnector,
3231
} from '@/hooks/queries/kb/connectors'
3332
import { useKnowledgeBasesQuery, useWorkspaceKnowledgeSearch } from '@/hooks/queries/kb/knowledge'
33+
import { useMemberAccessAvailable } from '@/hooks/use-member-access'
3434

3535
const EMPTY_MEMBER_CONNECTORS: WorkspaceMemberConnector[] = []
3636

@@ -185,13 +185,11 @@ export function KnowledgeSearchResults({
185185
isPlaceholderData,
186186
error,
187187
} = useWorkspaceKnowledgeSearch(workspaceId, knowledgeBaseIds, query)
188-
const { features } = useWorkspaceHostContext()
189188
/**
190-
* Judged by the workspace, as the server judges it: with per-member access
191-
* off, member-scoped documents are hidden, so no source is indexing anything
192-
* the viewer will see, and the list is not worth asking for.
189+
* With per-member access off, member-scoped documents are hidden, so the
190+
* indexing list is not worth asking for.
193191
*/
194-
const memberAccessAvailable = features?.knowledgeMemberAccess === true
192+
const memberAccessAvailable = useMemberAccessAvailable()
195193
const { data: memberConnectorRows } = useWorkspaceMemberConnectors(workspaceId, {
196194
enabled: memberAccessAvailable,
197195
})

apps/sim/app/workspace/[workspaceId]/home/components/search-sources/search-sources.tsx

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,14 @@ import {
1111
searchConnectorUnavailableReason,
1212
} from '@/lib/sim-search/connectors'
1313
import { SourceSetupModal } from '@/app/workspace/[workspaceId]/home/components/search-sources/source-setup-modal'
14-
import { useWorkspaceHostContext } from '@/app/workspace/[workspaceId]/providers/workspace-host-provider'
1514
import { BrandIcon } from '@/blocks/brand-icon'
1615
import {
1716
memberConnectorKeys,
1817
useWorkspaceMemberConnectors,
1918
type WorkspaceMemberConnector,
2019
} from '@/hooks/queries/kb/connectors'
2120
import { useWorkspacePermissionsQuery } from '@/hooks/queries/workspace'
21+
import { useMemberAccessAvailable } from '@/hooks/use-member-access'
2222
import { CONNECTABLE_MEMBERSHIPS, useMemberEnrollment } from '@/hooks/use-member-enrollment'
2323
import { usePermissionConfig } from '@/hooks/use-permission-config'
2424

@@ -140,12 +140,8 @@ interface SearchSourcesProps {
140140
*/
141141
export function SearchSources({ workspaceId }: SearchSourcesProps) {
142142
const { integrationAvailability } = usePermissionConfig()
143-
const { features } = useWorkspaceHostContext()
144-
/**
145-
* Judged by the workspace, as the server judges it: with per-member access
146-
* off, a connect is refused, so the chips say so instead of offering one.
147-
*/
148-
const memberAccessAvailable = features?.knowledgeMemberAccess === true
143+
/** With per-member access off, a connect is refused, so the chips say so instead. */
144+
const memberAccessAvailable = useMemberAccessAvailable()
149145
const { data: workspacePermissions } = useWorkspacePermissionsQuery(workspaceId)
150146
/** The first connect of a source turns it on for the workspace, which takes an admin. */
151147
const canCreate = workspacePermissions?.viewer?.isAdmin ?? false

apps/sim/app/workspace/[workspaceId]/home/components/user-input/components/mode-switcher/mode-switcher.test.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,8 @@ const mockUrlUpdate = vi.fn<(event: UrlUpdateEvent) => void>()
1515
vi.mock('next/navigation', () => ({
1616
useParams: () => ({ workspaceId: 'workspace-1' }),
1717
}))
18+
/** The switcher renders only where Search mode exists, so these tests are that workspace. */
19+
vi.mock('@/hooks/use-member-access', () => ({ useMemberAccessAvailable: () => true }))
1820
vi.mock('posthog-js/react', () => ({ usePostHog: () => null }))
1921
vi.mock('@/lib/posthog/client', () => ({ captureEvent: mockCaptureEvent }))
2022

apps/sim/app/workspace/[workspaceId]/home/components/user-input/user-input.test.tsx

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,19 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
99
import type { PromptEditorInstance } from '@/app/workspace/[workspaceId]/home/components/user-input/components/prompt-editor'
1010
import type { QueuedMessage } from '@/app/workspace/[workspaceId]/home/types'
1111

12-
const { mockSubmit, mockResetTranscript } = vi.hoisted(() => ({
12+
const { mockSubmit, mockResetTranscript, mockMemberAccessAvailable } = vi.hoisted(() => ({
1313
mockSubmit: vi.fn(),
1414
mockResetTranscript: vi.fn(),
15+
/** Search mode exists only where per-member access is on; these tests are that workspace. */
16+
mockMemberAccessAvailable: vi.fn(() => true),
1517
}))
1618

1719
vi.mock('next/navigation', () => ({ useParams: () => ({ workspaceId: 'workspace-1' }) }))
1820
vi.mock('posthog-js/react', () => ({ usePostHog: () => null }))
1921
vi.mock('@/lib/posthog/client', () => ({ captureEvent: vi.fn() }))
22+
vi.mock('@/hooks/use-member-access', () => ({
23+
useMemberAccessAvailable: () => mockMemberAccessAvailable(),
24+
}))
2025
vi.mock('@/hooks/use-settings-navigation', () => ({
2126
useSettingsNavigation: () => ({ navigateToSettings: vi.fn() }),
2227
}))

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

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,7 @@ import { useMarkMothershipChatRead } from '@/hooks/queries/mothership-chats'
6868
import { KNOWLEDGE_BASE_LIST_STALE_TIME, knowledgeKeys } from '@/hooks/queries/utils/knowledge-keys'
6969
import { useWorkflows } from '@/hooks/queries/workflows'
7070
import { getWorkspaceFilesQueryOptions, useWorkspaceFiles } from '@/hooks/queries/workspace-files'
71+
import { useMemberAccessAvailable } from '@/hooks/use-member-access'
7172
import { useOAuthReturnRouter } from '@/hooks/use-oauth-return'
7273
import type { ChatContext } from '@/stores/panel'
7374
import {
@@ -184,15 +185,19 @@ export function Home({ chatId, userName, userId }: HomeProps) {
184185
},
185186
[setSearchQueryParam, setSearchFilters]
186187
)
188+
const memberAccessAvailable = useMemberAccessAvailable()
187189
const [composerMode, setComposerMode] = useMothershipMode()
188190
/**
189191
* A link that carries a query but no mode opens in Search with the query in
190192
* the box; the composer follows the live query the same way (below), so the
191-
* box and the results never show two different queries.
193+
* box and the results never show two different queries. Where per-member
194+
* access is off there is no Search to open into, so the query stays a plain
195+
* Build draft rather than a mode write `useMothershipMode` would drop.
192196
*/
193197
useEffect(() => {
198+
if (!memberAccessAvailable) return
194199
if (searchQuery.trim() && composerMode === 'build') void setComposerMode('search')
195-
}, [searchQuery, composerMode, setComposerMode])
200+
}, [memberAccessAvailable, searchQuery, composerMode, setComposerMode])
196201
const hasCheckedLandingStorageRef = useRef(false)
197202
const initialViewInputRef = useRef<HTMLDivElement>(null)
198203
const initialViewUserInputRef = useRef<UserInputHandle>(null)
@@ -490,8 +495,13 @@ export function Home({ chatId, userName, userId }: HomeProps) {
490495
* Search lists documents, not a turn of the agent, and only a query can
491496
* be searched: attachments alone have nothing to search for. Assistant
492497
* makes the query a turn of the agent grounded in the sources.
498+
*
499+
* The override skips `useMothershipMode`, so the gate is applied again
500+
* where the mode is consumed: both modes answer from the workspace's
501+
* indexed sources, and neither is offered where those do not exist.
493502
*/
494-
const mode = modeOverride ?? composerMode
503+
const requestedMode = modeOverride ?? composerMode
504+
const mode = requestedMode !== 'build' && !memberAccessAvailable ? 'build' : requestedMode
495505
const answering = mode === 'assistant'
496506
if (mode === 'search') {
497507
/** A search sends nothing, so an edit in progress is released rather than left waiting. */
@@ -534,6 +544,7 @@ export function Home({ chatId, userName, userId }: HomeProps) {
534544
workspaceId,
535545
chatId,
536546
composerMode,
547+
memberAccessAvailable,
537548
editingQueuedId,
538549
cancelQueueEdit,
539550
prepareResourceViewForAgentTurn,
@@ -803,7 +814,7 @@ export function Home({ chatId, userName, userId }: HomeProps) {
803814
defaultValue={initialPrompt || searchQuery}
804815
draftScopeKey={draftScopeKey}
805816
onSubmit={handleSubmit}
806-
canSearch
817+
canSearch={memberAccessAvailable}
807818
clearOnSubmit={composerMode !== 'search'}
808819
onCleared={clearSearch}
809820
isSending={isSending}
@@ -835,7 +846,7 @@ export function Home({ chatId, userName, userId }: HomeProps) {
835846
isReconnecting={isReconnecting}
836847
isLoading={showChatSkeleton}
837848
onSubmit={handleSubmit}
838-
canSearch
849+
canSearch={memberAccessAvailable}
839850
clearOnSubmit={composerMode !== 'search'}
840851
onCleared={clearSearch}
841852
onStopGeneration={handleStopGeneration}
Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import { act } from 'react'
5+
import { NuqsTestingAdapter, type UrlUpdateEvent } from 'nuqs/adapters/testing'
6+
import { createRoot, type Root } from 'react-dom/client'
7+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
8+
import type { MothershipMode } from '@/app/workspace/[workspaceId]/home/search-params'
9+
10+
const { mockMemberAccessAvailable } = vi.hoisted(() => ({
11+
mockMemberAccessAvailable: vi.fn(() => true),
12+
}))
13+
const mockUrlUpdate = vi.fn<(event: UrlUpdateEvent) => void>()
14+
15+
vi.mock('@/hooks/use-member-access', () => ({
16+
useMemberAccessAvailable: () => mockMemberAccessAvailable(),
17+
}))
18+
19+
import { useMothershipMode } from '@/app/workspace/[workspaceId]/home/hooks/use-mothership-mode'
20+
21+
let root: Root | null = null
22+
let container: HTMLDivElement | null = null
23+
let current: ReturnType<typeof useMothershipMode> | null = null
24+
25+
function Probe() {
26+
current = useMothershipMode()
27+
return null
28+
}
29+
30+
function mount(searchParams = '') {
31+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
32+
container = document.createElement('div')
33+
document.body.appendChild(container)
34+
root = createRoot(container)
35+
act(() =>
36+
root?.render(
37+
<NuqsTestingAdapter hasMemory searchParams={searchParams} onUrlUpdate={mockUrlUpdate}>
38+
<Probe />
39+
</NuqsTestingAdapter>
40+
)
41+
)
42+
}
43+
44+
function mode(): MothershipMode {
45+
if (!current) throw new Error('Probe did not render')
46+
return current[0]
47+
}
48+
49+
/** nuqs batches its URL write onto a timeout, so a write is read back after the tick. */
50+
async function setMode(next: MothershipMode) {
51+
await act(async () => {
52+
current?.[1](next)
53+
await vi.advanceTimersByTimeAsync(1)
54+
})
55+
}
56+
57+
beforeEach(() => {
58+
vi.useFakeTimers({ toFake: ['setTimeout', 'clearTimeout'] })
59+
mockMemberAccessAvailable.mockReturnValue(true)
60+
mockUrlUpdate.mockClear()
61+
})
62+
63+
afterEach(() => {
64+
if (root) act(() => root?.unmount())
65+
container?.remove()
66+
root = null
67+
container = null
68+
current = null
69+
vi.useRealTimers()
70+
})
71+
72+
/**
73+
* The mode's ordinary read/write behavior is covered through the UI in
74+
* `mode-switcher.test.tsx`; one write stands here as the control the
75+
* per-member-access cases are read against.
76+
*/
77+
describe('useMothershipMode', () => {
78+
it('writes the chosen mode to the URL', async () => {
79+
mount()
80+
await setMode('search')
81+
82+
expect(mode()).toBe('search')
83+
expect(mockUrlUpdate.mock.lastCall?.[0].searchParams.get('mode')).toBe('search')
84+
})
85+
86+
describe('without per-member access', () => {
87+
beforeEach(() => {
88+
mockMemberAccessAvailable.mockReturnValue(false)
89+
})
90+
91+
it('reads Build from a link naming a mode the workspace does not have', () => {
92+
mount('?mode=search')
93+
94+
expect(mode()).toBe('build')
95+
})
96+
97+
it('writes no mode the workspace does not have', async () => {
98+
mount()
99+
await setMode('search')
100+
101+
expect(mode()).toBe('build')
102+
expect(mockUrlUpdate).not.toHaveBeenCalled()
103+
})
104+
105+
it('still returns to Build, so a stale link can be left', async () => {
106+
mount('?mode=search&q=budget')
107+
await setMode('build')
108+
109+
expect(mode()).toBe('build')
110+
expect(mockUrlUpdate.mock.lastCall?.[0].searchParams.toString()).toBe('')
111+
})
112+
})
113+
})

0 commit comments

Comments
 (0)