Skip to content

Commit d381153

Browse files
committed
improvement(url-state): platform-wide sweep — migrate the last three view-state stragglers, codify conventions
Sweep across landing, workspace, and settings surfaces found only three remaining candidates (everything else verified clean or correctly non-URL): - knowledge document page: chunk enabled-status filter joins page/search/sort in the URL (?enabled=), resetting page in the same write - workflow-mcp-servers: detail Details/Workflows tab deep-links via ?server-tab, cleared alongside the server id on close - byok: provider search binds the shared settings ?search= via a controlled prop pair (modal/embedded consumers keep local state) Rule updates (.claude/rules/sim-url-state.md): shallow defaults documented, urlKeys kebab remapping, throttleMs deprecation, startTransition with shallow:false, shared-parser null-fallback rule, resolve-before-open gating, close-with-replace, and the reusable-component controlled-search pattern.
1 parent 3fa4b4f commit d381153

7 files changed

Lines changed: 92 additions & 21 deletions

File tree

.claude/rules/sim-url-state.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -50,11 +50,13 @@ Co-locate a `search-params.ts` next to the feature. Export the parser map (and s
5050

5151
Conventions:
5252

53-
- `.withDefault(...)` on every parser so reads are non-null.
54-
- Filter / search / toggle / pagination options: `{ history: 'replace', shallow: true, clearOnDefault: true }` — clean URLs, no back-stack churn.
53+
- `.withDefault(...)` on every parser so reads are non-null. A deliberately **nullable** parser (dynamic default, custom-range-only dates, nullable sort) must carry a comment saying why.
54+
- Filter / search / toggle / pagination options: `{ history: 'replace', clearOnDefault: true }` — clean URLs, no back-stack churn. `shallow` defaults to `true` in nuqs; writing `shallow: true` explicitly is fine but not required.
5555
- Navigations that belong in browser history (changing folder, opening a deep-linked entity): `{ history: 'push' }`.
56-
- `shallow: false` **only** when a Server Component / loader must re-read the param.
57-
- Short, stable, **kebab-case** URL keys. Renaming a key is a breaking change to shared links — treat it as one.
56+
- `shallow: false` **only** when a Server Component / loader must re-read the param. For loading states during the server re-render, pass React's `startTransition` via `.withOptions({ startTransition, shallow: false })`.
57+
- Short, stable, **kebab-case** URL keys. Renaming a key is a breaking change to shared links — treat it as one. When the parser-map key is camelCase (for clean destructuring), remap the wire key via the `urlKeys` option in the shared options object (see `files/search-params.ts` `uploadedBy: 'uploaded-by'`, `ee/audit-logs/search-params.ts` `timeRange: 'time-range'`); nuqs also exports a `UrlKeys<typeof parsers>` type helper for standalone mappings.
58+
- `throttleMs` is deprecated in nuqs — rate-limit URL writes with `limitUrlUpdates: throttle(ms)` / `debounce(ms)` (the debounced-search hook below already does this).
59+
- A parser **shared across surfaces with different defaults** (e.g. `parseAsTimeRange`) must `parse` unknown tokens to `null` — never to one surface's default — so each consumer's `.withDefault(...)` decides the fallback.
5860
- For an opaque/literal value use `parseAsStringLiteral([...] as const)`; for a custom wire format use [`createParser`](https://nuqs.dev/docs/parsers).
5961
- A `createParser` for a value **not** comparable with `===` (arrays, objects, `Date`) **must** define an `eq``clearOnDefault` uses it to detect the default, so without it an empty-array/object default never strips from the URL. Built-in `parseAsArrayOf(...)` already ships its own `eq`; only string/number/boolean custom parsers can omit it. Example (array): `eq: (a, b) => a.length === b.length && a.every((v, i) => v === b[i])`.
6062

@@ -200,7 +202,11 @@ const [skillId, setSkillId] = useQueryState(skillIdParam.key, {
200202
const editingSkill = skillId ? (skills.find((s) => s.id === skillId) ?? null) : null
201203
```
202204

203-
Open the panel/modal when the id resolves to a loaded entity; closing it calls `setSkillId(null)`. Because this reads `useSearchParams` it needs a **Suspense** boundary on the page (see below). A separate "create new" flow has no id and stays in local `useState`.
205+
Open the panel/modal only when the id **resolves to a loaded entity** — never gate on the raw param alone, or a dead/stale id (deleted entity, old bookmark) renders a broken detail view and a still-loading list flashes one. A dead id simply falls back to the list; the lingering param is harmless. Because this reads `useSearchParams` it needs a **Suspense** boundary on the page (see below). A separate "create new" flow has no id and stays in local `useState`.
206+
207+
**Close with `replace`, open with `push`.** Opening pushed a history entry; closing must not push another. Close via the setter's per-call options — `setSkillId(null, { history: 'replace' })` — so Back from the list leaves the page instead of reopening the detail (see `mcp.tsx`, `workflow-mcp-servers.tsx`, access-control, custom-blocks, forks). Secondary params scoped to the detail view (e.g. its active tab, `server-tab`) are cleared in the same close handler with their own setter — nuqs batches same-tick writes into one URL update.
208+
209+
**Reusable components** rendered both as a settings/list page and inside a modal (e.g. `BYOKKeyManager`) expose an optional controlled `searchTerm`/`onSearchTermChange` prop pair: the page consumer binds the URL (`useSettingsSearch()`), modal consumers omit the props and keep local state. Never bind URL state from inside a component that can mount in a non-destination context.
204210

205211
## Read-then-strip deep links
206212

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/document.tsx

Lines changed: 28 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,12 @@ export function Document({
137137
const { workspaceId } = useParams()
138138
const router = useRouter()
139139
const [
140-
{ page: currentPageFromURL, chunk: chunkFromURL, search: searchQuery },
140+
{
141+
page: currentPageFromURL,
142+
chunk: chunkFromURL,
143+
search: searchQuery,
144+
enabled: enabledFilterParam,
145+
},
141146
setDocumentParams,
142147
] = useQueryStates(documentParsers, documentUrlKeys)
143148
const userPermissions = useUserPermissionsContext()
@@ -160,16 +165,31 @@ export function Document({
160165
)
161166
/** Raw URL value drives the input; the chunk search query always sees it trimmed. */
162167
const debouncedSearchQuery = useDebounce(searchQuery, CHUNK_SEARCH_DEBOUNCE_MS).trim()
163-
const [enabledFilter, setEnabledFilter] = useState<string[]>([])
164168
const {
165169
activeSort,
166170
onSort: onSortColumn,
167171
onClear: onClearSort,
168172
} = useUrlSort(documentChunkSortParams, documentUrlKeys)
169173

170-
const enabledFilterParam = useMemo(
171-
() => (enabledFilter.length === 1 ? (enabledFilter[0] as 'enabled' | 'disabled') : 'all'),
172-
[enabledFilter]
174+
/** Multi-select UI view of the scalar `enabled` param (`all` ↔ nothing selected). */
175+
const enabledFilter = useMemo<string[]>(
176+
() => (enabledFilterParam === 'all' ? [] : [enabledFilterParam]),
177+
[enabledFilterParam]
178+
)
179+
180+
/**
181+
* Collapses the dropdown's multi-select values to the scalar param (one value
182+
* filters; none or both mean `all`) and resets `page` in the same write so a
183+
* filter change always lands on the first page.
184+
*/
185+
const setEnabledFilter = useCallback(
186+
(values: string[]) => {
187+
void setDocumentParams({
188+
enabled: values.length === 1 ? (values[0] as 'enabled' | 'disabled') : null,
189+
page: null,
190+
})
191+
},
192+
[setDocumentParams]
173193
)
174194

175195
const {
@@ -638,7 +658,6 @@ export function Document({
638658
onMultiSelectChange={(values) => {
639659
setEnabledFilter(values)
640660
setSelectedChunks(new Set())
641-
void goToPage(1)
642661
}}
643662
overlayContent={
644663
<span className='truncate text-[var(--text-primary)]'>{enabledDisplayLabel}</span>
@@ -654,7 +673,6 @@ export function Document({
654673
onClick={() => {
655674
setEnabledFilter([])
656675
setSelectedChunks(new Set())
657-
void goToPage(1)
658676
}}
659677
className='flex h-[32px] w-full items-center justify-center rounded-md text-[var(--text-secondary)] text-caption transition-colors hover-hover:bg-[var(--surface-active)]'
660678
>
@@ -663,20 +681,19 @@ export function Document({
663681
)}
664682
</div>
665683
),
666-
[enabledFilter, enabledDisplayLabel, goToPage]
684+
[enabledFilter, enabledDisplayLabel, setEnabledFilter]
667685
)
668686

669687
const filterTags: FilterTag[] = useMemo(
670688
() =>
671689
enabledFilter.map((value) => ({
672690
label: `Status: ${value === 'enabled' ? 'Enabled' : 'Disabled'}`,
673691
onRemove: () => {
674-
setEnabledFilter((prev) => prev.filter((v) => v !== value))
692+
setEnabledFilter(enabledFilter.filter((v) => v !== value))
675693
setSelectedChunks(new Set())
676-
void goToPage(1)
677694
},
678695
})),
679-
[enabledFilter, goToPage]
696+
[enabledFilter, setEnabledFilter]
680697
)
681698

682699
const handleChunkClick = useCallback(

apps/sim/app/workspace/[workspaceId]/knowledge/[id]/[documentId]/search-params.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
1-
import { parseAsInteger, parseAsString } from 'nuqs/server'
1+
import { parseAsInteger, parseAsString, parseAsStringLiteral } from 'nuqs/server'
22
import { createSortParams } from '@/lib/url-state'
33

4+
/** Chunk `enabled` filter buckets, matching the status filter dropdown. */
5+
const ENABLED_FILTERS = ['all', 'enabled', 'disabled'] as const
6+
47
/** Sortable chunk columns, matching the `Resource.Options` sort menu ids. */
58
export const CHUNK_SORT_COLUMNS = ['index', 'tokens', 'status'] as const
69

@@ -24,11 +27,14 @@ export const documentChunkSortParams = createSortParams(CHUNK_SORT_COLUMNS)
2427
* - `search` is the chunk content search. The input is controlled directly by
2528
* the instant nuqs value; only its URL write is debounced via
2629
* `useDebouncedSearchSetter` — never written on every keystroke.
30+
* - `enabled` filters chunks by enabled status (`all` clears from the URL),
31+
* mirroring the same filter on the knowledge base document list.
2732
*/
2833
export const documentParsers = {
2934
page: parseAsInteger.withDefault(1),
3035
chunk: parseAsString,
3136
search: parseAsString.withDefault(''),
37+
enabled: parseAsStringLiteral(ENABLED_FILTERS).withDefault('all'),
3238
} as const
3339

3440
/**

apps/sim/app/workspace/[workspaceId]/settings/[section]/search-params.ts

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,22 @@ export const forkViewUrlKeys = {
4949
clearOnDefault: true,
5050
} as const
5151

52+
/**
53+
* `server-tab` is the active tab (Details / Workflows) inside the deep-linked
54+
* workflow MCP server detail view, so a shared `mcpServerId` link can land on
55+
* either tab.
56+
*/
57+
export const serverTabParam = {
58+
key: 'server-tab',
59+
parser: parseAsStringLiteral(['details', 'workflows'] as const).withDefault('details'),
60+
} as const
61+
62+
/** Tab view-state: clean URLs, no back-stack churn. */
63+
export const serverTabUrlKeys = {
64+
history: 'replace',
65+
clearOnDefault: true,
66+
} as const
67+
5268
/**
5369
* `group-id` deep-links the Access Control settings tab to a specific
5470
* permission group's detail sub-view (mirrors `mcpServerId` on the MCP tab).

apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager.tsx

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,13 @@ interface BYOKKeyManagerBaseProps {
6666
description?: string
6767
/** Show the provider search box (hidden when there are only a couple). */
6868
showSearch?: boolean
69+
/**
70+
* Controlled search value + setter. The BYOK settings page passes the shared
71+
* `?search=` binding (`useSettingsSearch`) so the search is deep-linkable;
72+
* modal/embedded consumers omit both and keep local state.
73+
*/
74+
searchTerm?: string
75+
onSearchTermChange?: (value: string) => void
6976
}
7077

7178
/** One key per provider; saving replaces the stored key. */
@@ -138,7 +145,9 @@ export function BYOKKeyManager(props: BYOKKeyManagerProps) {
138145
showSearch = true,
139146
} = props
140147

141-
const [searchTerm, setSearchTerm] = useState('')
148+
const [localSearchTerm, setLocalSearchTerm] = useState('')
149+
const searchTerm = props.searchTerm ?? localSearchTerm
150+
const setSearchTerm = props.onSearchTermChange ?? setLocalSearchTerm
142151
const [editing, setEditing] = useState<EditingState | null>(null)
143152
const [apiKeyInput, setApiKeyInput] = useState('')
144153
const [nameInput, setNameInput] = useState('')

apps/sim/app/workspace/[workspaceId]/settings/components/byok/byok.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,7 @@ import {
4848
type BYOKProviderSection,
4949
} from '@/app/workspace/[workspaceId]/settings/components/byok/byok-key-manager'
5050
import { SettingsPanel } from '@/app/workspace/[workspaceId]/settings/components/settings-panel'
51+
import { useSettingsSearch } from '@/app/workspace/[workspaceId]/settings/components/use-settings-search'
5152
import { useBYOKKeys, useDeleteBYOKKey, useUpsertBYOKKey } from '@/hooks/queries/byok-keys'
5253
import type { BYOKProviderId } from '@/tools/types'
5354

@@ -354,6 +355,7 @@ export function BYOK() {
354355
const workspaceId = (params?.workspaceId as string) || ''
355356
const workspacePermissions = useUserPermissionsContext()
356357
const canManage = canMutateWorkspaceSettingsSection('byok', workspacePermissions)
358+
const [searchTerm, setSearchTerm] = useSettingsSearch()
357359

358360
const { data, isLoading } = useBYOKKeys(workspaceId)
359361
const upsertKey = useUpsertBYOKKey()
@@ -381,6 +383,8 @@ export function BYOK() {
381383
isSaving={upsertKey.isPending}
382384
isDeleting={deleteKey.isPending}
383385
readOnly={!canManage}
386+
searchTerm={searchTerm}
387+
onSearchTermChange={setSearchTerm}
384388
onSaveKey={async ({ providerId, apiKey, keyId, name }) => {
385389
await upsertKey.mutateAsync({
386390
workspaceId,

apps/sim/app/workspace/[workspaceId]/settings/components/workflow-mcp-servers/workflow-mcp-servers.tsx

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,8 @@ import { useUserPermissionsContext } from '@/app/workspace/[workspaceId]/provide
3232
import {
3333
mcpServerIdParam,
3434
mcpServerIdUrlKeys,
35+
serverTabParam,
36+
serverTabUrlKeys,
3537
} from '@/app/workspace/[workspaceId]/settings/[section]/search-params'
3638
import { CreateApiKeyModal } from '@/app/workspace/[workspaceId]/settings/components/api-keys/components'
3739
import { RowActionsMenu } from '@/app/workspace/[workspaceId]/settings/components/row-actions-menu'
@@ -108,7 +110,10 @@ function ServerDetailView({ canManage, workspaceId, serverId, onBack }: ServerDe
108110
const [editServerName, setEditServerName] = useState('')
109111
const [editServerDescription, setEditServerDescription] = useState('')
110112
const [editServerIsPublic, setEditServerIsPublic] = useState(false)
111-
const [activeServerTab, setActiveServerTab] = useState<'workflows' | 'details'>('details')
113+
const [activeServerTab, setActiveServerTab] = useQueryState(serverTabParam.key, {
114+
...serverTabParam.parser,
115+
...serverTabUrlKeys,
116+
})
112117

113118
const [selectedWorkflowId, setSelectedWorkflowId] = useState<string | null>(null)
114119

@@ -401,7 +406,7 @@ function ServerDetailView({ canManage, workspaceId, serverId, onBack }: ServerDe
401406
{ value: 'workflows', label: 'Workflows' },
402407
]}
403408
value={activeServerTab}
404-
onChange={(value) => setActiveServerTab(value as 'workflows' | 'details')}
409+
onChange={(value) => void setActiveServerTab(value as 'workflows' | 'details')}
405410
/>
406411

407412
<div className='min-h-[300px] pt-4'>
@@ -892,6 +897,11 @@ export function WorkflowMcpServers() {
892897
})
893898
const [serverToDelete, setServerToDelete] = useState<WorkflowMcpServer | null>(null)
894899
const [deletingServers, setDeletingServers] = useState<Set<string>>(() => new Set())
900+
/** Cleared alongside the server id on close so the tab never lingers on the list URL. */
901+
const [, setServerTab] = useQueryState(serverTabParam.key, {
902+
...serverTabParam.parser,
903+
...serverTabUrlKeys,
904+
})
895905

896906
const filteredServers = useMemo(() => {
897907
if (!searchTerm.trim()) return servers
@@ -948,7 +958,10 @@ export function WorkflowMcpServers() {
948958
canManage={canAdmin}
949959
workspaceId={workspaceId}
950960
serverId={selectedServerId}
951-
onBack={() => setSelectedServerId(null, { history: 'replace' })}
961+
onBack={() => {
962+
void setServerTab(null, { history: 'replace' })
963+
void setSelectedServerId(null, { history: 'replace' })
964+
}}
952965
/>
953966
)
954967
}

0 commit comments

Comments
 (0)