Skip to content

Commit 03f1641

Browse files
committed
fix(url-state): verification-round fixes — reject unparseable date params, tighten docs
- new parseAsDateString parser (logs + audit-logs): an unparseable ?start-date=/?end-date= now parses to null (missing bound) instead of crashing the audit-logs render via Invalid Date .toISOString(), and hardens the same class in logs - audit-logs: remove the provably dead cancel-revert branch and its ref — the URL only holds 'Custom range' after Apply writes both bounds atomically - workflow-mcp-servers: reset a lingering ?server-tab= when opening a server so a dead deep link can't re-target the next open - knowledge document: drop the unreachable 'N selected' label branch - sim-url-state.md fact-check corrections: cover apps/sim/ee in paths, focusedBlockId -> currentBlockId (the real store field), note that history/clearOnDefault are nuqs v2 defaults, fix the Suspense cross-reference and parseAsIsoDate serialize detail, clarify the *UrlKeys naming convention; list byok in the shared-search consumer docs
1 parent 138822a commit 03f1641

8 files changed

Lines changed: 49 additions & 25 deletions

File tree

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

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ paths:
33
- "apps/sim/app/**/*.tsx"
44
- "apps/sim/app/**/*.ts"
55
- "apps/sim/app/**/search-params.ts"
6+
- "apps/sim/ee/**/*.tsx"
7+
- "apps/sim/ee/**/*.ts"
68
---
79

810
# URL / Query-Param State (nuqs)
@@ -51,7 +53,7 @@ Co-locate a `search-params.ts` next to the feature. Export the parser map (and s
5153
Conventions:
5254

5355
- `.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.
56+
- Filter / search / toggle / pagination options: `{ history: 'replace', clearOnDefault: true }` — clean URLs, no back-stack churn. Note all three of `history: 'replace'`, `clearOnDefault: true`, and `shallow: true` are already the nuqs v2 defaults — writing the first two explicitly is documentation (and guards the groups whose options differ, e.g. `history: 'push'`), and `shallow: true` may be omitted entirely.
5557
- Navigations that belong in browser history (changing folder, opening a deep-linked entity): `{ history: 'push' }`.
5658
- `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 })`.
5759
- 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.
@@ -77,11 +79,12 @@ export const thingsParsers = {
7779
/** Clean URLs, no back-stack churn for filter changes. */
7880
export const thingsUrlKeys = {
7981
history: 'replace',
80-
shallow: true,
8182
clearOnDefault: true,
8283
} as const
8384
```
8485

86+
(The `*UrlKeys` suffix is the repo's naming convention for a feature's shared **options** object — which may itself contain a nuqs `urlKeys` key-remapping entry; the two are different things.)
87+
8588
### Client — `useQueryStates` (grouped) / `useQueryState` (single)
8689

8790
```typescript
@@ -184,7 +187,7 @@ Sort params live alongside — not inside — the feature's grouped filter parse
184187

185188
A date-only param (a calendar anchor, a date filter) is stored as `yyyy-MM-dd` — never serialize a full `Date`/timestamp when only the day matters.
186189

187-
**Local vs UTC — pick the parser that matches your date math.** nuqs's built-in `parseAsIsoDate` is **UTC-based** (`serialize` via `toISOString()`, `parse` to UTC midnight). If your `Date` is local-time (e.g. produced by local-time helpers and read by `date-fns` `startOfWeek`/`isSameDay`, which are all local), `parseAsIsoDate` will shift the day by ±1 in any non-UTC timezone on reload/deep-link/back-forward. For local-time date math, use a small local-date `createParser` that serializes/parses on local calendar fields (`getFullYear`/`getMonth`/`getDate``new Date(y, m-1, d)`) with an `eq` comparing y/m/d. Only use `parseAsIsoDate` when the value is genuinely UTC/midnight-UTC. See `scheduled-tasks/search-params.ts` (`parseAsLocalDate`).
190+
**Local vs UTC — pick the parser that matches your date math.** nuqs's built-in `parseAsIsoDate` is **UTC-based** (`serialize` via `toISOString().slice(0, 10)`, `parse` to UTC midnight). If your `Date` is local-time (e.g. produced by local-time helpers and read by `date-fns` `startOfWeek`/`isSameDay`, which are all local), `parseAsIsoDate` will shift the day by ±1 in any non-UTC timezone on reload/deep-link/back-forward. For local-time date math, use a small local-date `createParser` that serializes/parses on local calendar fields (`getFullYear`/`getMonth`/`getDate``new Date(y, m-1, d)`) with an `eq` comparing y/m/d. Only use `parseAsIsoDate` when the value is genuinely UTC/midnight-UTC. See `scheduled-tasks/search-params.ts` (`parseAsLocalDate`).
188191

189192
When the default is **dynamic** (e.g. "today"), make the param **nullable** (omit `.withDefault`) and derive the fallback in the hook (`const anchor = param ?? today`), so a clean URL means the dynamic default and navigating back to it writes `null` (clears the param). See `scheduled-tasks/hooks/use-calendar.ts`.
190193

@@ -202,7 +205,7 @@ const [skillId, setSkillId] = useQueryState(skillIdParam.key, {
202205
const editingSkill = skillId ? (skills.find((s) => s.id === skillId) ?? null) : null
203206
```
204207

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`.
208+
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 "Suspense boundary" above). A separate "create new" flow has no id and stays in local `useState`.
206209

207210
**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.
208211

@@ -223,8 +226,8 @@ The workflow editor (`apps/sim/app/workspace/[workspaceId]/w/**`) is realtime/so
223226

224227
Borderline candidates that *look* shareable but currently stay in Zustand because moving them fights existing machinery:
225228

226-
- **Panel `activeTab`** and **`canvasMode`**persisted local *preferences* wired into an SSR flash-prevention path (`data-panel-active-tab` + `_hasHydrated`). They are layout prefs, not destinations; moving them would unwind the SSR machinery and risk tab-flash on load.
227-
- **`focusedBlockId`** ("look at this block") — the only genuinely shareable candidate, but it is entangled with the persisted editor store and panel-open orchestration. Adding it is a *new feature*, not a migration; ship it deliberately (with runtime verification against a live socket), not as part of a sweep.
229+
- **Panel `activeTab`** — a persisted local *preference* wired into an SSR flash-prevention path (`data-panel-active-tab` + `_hasHydrated`); moving it would unwind that machinery and risk tab-flash on load. **Canvas mode** (`mode` on `useCanvasModeStore`) is likewise a persisted layout preference, not a destination.
230+
- **The panel editor's `currentBlockId`** (`stores/panel/editor/store.ts` — a would-be "look at this block" deep link) — the only genuinely shareable candidate, but it is persisted and entangled with panel-open orchestration. Adding a URL param for it is a *new feature*, not a migration; ship it deliberately (with runtime verification against a live socket), not as part of a sweep.
228231

229232
Rule of thumb for the editor: if state is socket-coupled, high-frequency, viewport-related, or a persisted resize/preference, it stays in Zustand. When in doubt, leave it and flag it — do not force fragile URL state into the canvas.
230233

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -639,8 +639,7 @@ export function Document({
639639

640640
const enabledDisplayLabel = useMemo(() => {
641641
if (enabledFilter.length === 0) return 'All'
642-
if (enabledFilter.length === 1) return enabledFilter[0] === 'enabled' ? 'Enabled' : 'Disabled'
643-
return `${enabledFilter.length} selected`
642+
return enabledFilter[0] === 'enabled' ? 'Enabled' : 'Disabled'
644643
}, [enabledFilter])
645644

646645
const filterContent = useMemo(

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

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,21 @@ export const parseAsLogLevel = createParser<LogLevel>({
7777
},
7878
})
7979

80+
/**
81+
* Parser for free-form date/datetime strings (`startDate`/`endDate`). Rejects
82+
* unparseable values at the URL boundary — an invalid date string reaching
83+
* `new Date(...).toISOString()` throws, so a malformed deep link must parse to
84+
* `null` (treated as a missing bound) instead of crashing the consumer.
85+
*/
86+
export const parseAsDateString = createParser<string>({
87+
parse(value) {
88+
return Number.isNaN(Date.parse(value)) ? null : value
89+
},
90+
serialize(value) {
91+
return value
92+
},
93+
})
94+
8095
const CORE_TRIGGER_SET = new Set<string>(CORE_TRIGGER_TYPES)
8196

8297
/**
@@ -109,8 +124,8 @@ export const logFilterParsers = {
109124
* Deliberately nullable: only populated when timeRange is "Custom range";
110125
* every preset range derives its window from the label instead.
111126
*/
112-
startDate: parseAsString,
113-
endDate: parseAsString,
127+
startDate: parseAsDateString,
128+
endDate: parseAsDateString,
114129
level: parseAsLogLevel.withDefault('all'),
115130
workflowIds: parseAsArrayOf(parseAsString).withDefault([]),
116131
folderIds: parseAsArrayOf(parseAsString).withDefault([]),

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { parseAsString } from 'nuqs/server'
22

33
/**
44
* Shared URL query-param definition for the settings list search boxes
5-
* (teammates, api-keys, copilot, custom-tools, mcp, secrets,
5+
* (teammates, api-keys, byok, copilot, custom-tools, mcp, secrets,
66
* workflow-mcp-servers, and the ee sections: audit-logs, access-control,
77
* custom-blocks, data-drains, forks). Settings sections never co-render, so
88
* they all share the `search` key without collisions.

apps/sim/app/workspace/[workspaceId]/settings/components/use-settings-search.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,8 +9,9 @@ import { useDebouncedSearchSetter } from '@/hooks/use-debounced-search-setter'
99

1010
/**
1111
* The shared `?search=` binding for settings list search boxes (teammates,
12-
* api-keys, copilot, custom-tools, mcp, secrets, workflow-mcp-servers, and the
13-
* ee sections: audit-logs, access-control, custom-blocks, data-drains, forks).
12+
* api-keys, byok, copilot, custom-tools, mcp, secrets, workflow-mcp-servers,
13+
* and the ee sections: audit-logs, access-control, custom-blocks, data-drains,
14+
* forks).
1415
* Composes `useDebouncedSearchSetter`, so it carries the canonical semantics:
1516
* the value updates instantly (drives the controlled input and the in-memory
1617
* filter), non-empty URL writes are debounced, and clearing (or a

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

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1024,7 +1024,14 @@ export function WorkflowMcpServers() {
10241024
<RowActionsMenu
10251025
label='Server actions'
10261026
actions={[
1027-
{ label: 'Details', onSelect: () => setSelectedServerId(server.id) },
1027+
{
1028+
label: 'Details',
1029+
onSelect: () => {
1030+
// A lingering ?server-tab= (dead deep link) must not re-target the next open — reset it in the same batched push.
1031+
void setServerTab(null)
1032+
void setSelectedServerId(server.id)
1033+
},
1034+
},
10281035
...(canAdmin
10291036
? [
10301037
{

apps/sim/ee/audit-logs/components/audit-logs.tsx

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,10 +251,6 @@ export function AuditLogs({ organizationId }: AuditLogsProps) {
251251
? DEFAULT_AUDIT_TIME_RANGE
252252
: urlFilters.timeRange
253253
const [datePickerOpen, setDatePickerOpen] = useState(false)
254-
/** Cancel target for the date picker — never 'Custom range' itself, so a deep link straight to an empty custom range still cancels back to a preset. */
255-
const previousTimeRangeRef = useRef<TimeRange>(
256-
timeRange === 'Custom range' ? DEFAULT_AUDIT_TIME_RANGE : timeRange
257-
)
258254
const dateRangeAppliedRef = useRef(false)
259255
const [searchTerm, setSearchTerm] = useSettingsSearch()
260256
const debouncedSearch = useDebounce(searchTerm, SEARCH_DEBOUNCE_MS).trim()
@@ -307,7 +303,6 @@ export function AuditLogs({ organizationId }: AuditLogsProps) {
307303

308304
const handleTimeRangeChange = (value: string) => {
309305
if (value === 'Custom range') {
310-
previousTimeRangeRef.current = timeRange
311306
setDatePickerOpen(true)
312307
} else {
313308
void setUrlFilters({ timeRange: value as TimeRange, startDate: null, endDate: null })
@@ -320,10 +315,11 @@ export function AuditLogs({ organizationId }: AuditLogsProps) {
320315
setDatePickerOpen(false)
321316
}
322317

318+
/**
319+
* Cancel is a pure close: the URL only ever holds 'Custom range' after Apply
320+
* wrote both bounds atomically, so there is never a pending state to revert.
321+
*/
323322
const handleDatePickerCancel = () => {
324-
if (timeRange === 'Custom range' && !customStartDate) {
325-
void setUrlFilters({ timeRange: previousTimeRangeRef.current })
326-
}
327323
setDatePickerOpen(false)
328324
}
329325

apps/sim/ee/audit-logs/search-params.ts

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,8 @@
11
import { parseAsArrayOf, parseAsString } from 'nuqs/server'
2-
import { parseAsTimeRange } from '@/app/workspace/[workspaceId]/logs/search-params'
2+
import {
3+
parseAsDateString,
4+
parseAsTimeRange,
5+
} from '@/app/workspace/[workspaceId]/logs/search-params'
36
import type { TimeRange } from '@/stores/logs/filters/types'
47

58
export const DEFAULT_AUDIT_TIME_RANGE: TimeRange = 'Past 30 days'
@@ -18,8 +21,8 @@ export const DEFAULT_AUDIT_TIME_RANGE: TimeRange = 'Past 30 days'
1821
export const auditLogFilterParsers = {
1922
types: parseAsArrayOf(parseAsString).withDefault([]),
2023
timeRange: parseAsTimeRange.withDefault(DEFAULT_AUDIT_TIME_RANGE),
21-
startDate: parseAsString,
22-
endDate: parseAsString,
24+
startDate: parseAsDateString,
25+
endDate: parseAsDateString,
2326
} as const
2427

2528
/** Filter view-state: clean URLs, no back-stack churn, kebab-case URL keys. */

0 commit comments

Comments
 (0)