Skip to content

Commit d8da1e2

Browse files
authored
fix(state): align server/client state with best practices (query-key bugs, persist hygiene, useState) (#5166)
* fix(queries): close React Query key/fetch-arg drift cache collisions Several query hooks fetched with an identifier that was absent from their queryKey, so distinct fetch args shared one cache entry. Thread the missing args into the key factories and update all callsites/invalidations. - organization: useOrganization always fetched the ACTIVE org via getFullOrganization() while caching under detail(orgId). Pass orgId through to the better-auth call (query.organizationId); active-org behavior unchanged. - logs: logKeys.detail now keys on (workspaceId, logId) to prevent cross- workspace collision; updated useLogDetail, useLogByExecutionId, prefetchLogDetail, useCancelExecution optimistic path, and external callsites. - inbox: inboxKeys.taskList now includes cursor/limit (pagination args were sent but omitted from the key); keepPreviousData pagination UX preserved. - a2a: narrow create/update byWorkflows() invalidation to byWorkflow(ws, wf) since their responses reliably carry both ids; delete/publish stay broad. Not bugs (verified, left unchanged): - kb/connectors update/delete invalidate knowledgeKeys.detail(kbId), which is a prefix of connectorKeys.all(kbId) — connector list/detail are invalidated transitively by React Query prefix matching. Harness: add a key-fetch-arg-drift check to check-react-query-patterns.ts that flags a camelCase identifier the queryFn forwards into the fetch but is absent from the queryKey (excludes the requestJson contract arg, PascalCase/SCREAMING constants, and signal/pageParam machinery). Document the rule in sim-queries.md. tables.useTable annotated rq-lint-allow (tableId globally unique; workspaceId is only an authz scope). * fix(stores): whitelist durable fields in persist partialize chat/terminal/panel persist configs leaked actions and transient state into localStorage. Replace the chat full-state spread with an explicit durable whitelist, and add partialize to terminal and panel (which had none) so isResizing and _hasHydrated are no longer persisted. Panel keeps activeTab + panelWidth because the layout.tsx blocking script reads them from panel-state to set data-panel-active-tab before hydration (SSR tab-flash prevention). Harden sim-stores doctrine: persist MUST use an explicit partialize whitelist; never persist transient flags or _hasHydrated. * fix(state): model component useState as single source of truth - edit-knowledge-base-modal: reset fields on closed→open via prevOpenRef render idiom instead of mirroring props into state through useEffect (a prop change while open no longer clobbers in-progress edits) - use-verification: collapse contradictory isLoading/isVerified/isInvalidOtp booleans into a single status enum + errorMessage; consumer derives flags - contact-form / demo-request-modal: derive busy/success from the mutation object; delete duplicated submitSuccess local state - sim-hooks.md: add state-shape rule (no props-into-state, status enum, derive mutation state) * fix(verify): clear lingering message on complete OTP (restore parity) * docs(state): convert inline reset comment to TSDoc * docs(state): tighten harness rules for accuracy (queryFn forwards, partialize whitelist, mutation-flag caveat) * fix(verify): block auto-verify while a resend is in flight (restore parity) * fix(logs): key cancel optimistic detail by route workspaceId (not the log row)
1 parent 951ad42 commit d8da1e2

19 files changed

Lines changed: 307 additions & 76 deletions

File tree

.claude/rules/sim-hooks.md

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,3 +48,19 @@ export function useFeature({ id, onSelect }: UseFeatureProps) {
4848
4. Wrap returned functions in useCallback
4949
5. Server data goes through React Query (`hooks/queries/`), never `useState` + `fetch`
5050
6. Keep only UI/orchestration state in these hooks
51+
52+
## State shape
53+
54+
Never mirror a prop into state with `useState(prop)` + a syncing `useEffect` — a prop change clobbers in-progress local edits. Use the prop directly, reset via a remount `key`, or — when you must seed local state from a prop only on a transition (e.g. a modal opening) — reset during render with the `prevX` ref idiom:
55+
56+
```typescript
57+
const prevOpenRef = useRef(open)
58+
if (prevOpenRef.current !== open) {
59+
prevOpenRef.current = open
60+
if (open) setName(initialName) // closed → open only
61+
}
62+
```
63+
64+
Model mutually-exclusive flags as ONE `status` enum, not several contradictory booleans. `isLoading`/`isVerified`/`isInvalidOtp` describing one machine collapse to `status: 'idle' | 'verifying' | 'verified' | 'error'` (+ `errorMessage`); derive any boolean a consumer still needs (`status === 'error'`).
65+
66+
Derive busy/success from the mutation object — never duplicate `mutation.isPending`/`mutation.isSuccess` into local `useState`. Read them directly (`mutation.isSuccess`) and reset with `mutation.reset()`. A distinct phase the mutation doesn't cover — e.g. a pre-submit captcha/Turnstile gate that runs before `mutate()` — is not a duplicate; keep that flag.

.claude/rules/sim-queries.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,8 @@ export const entityKeys = {
2525

2626
Never use inline query keys — always use the factory.
2727

28+
**Every identifier the `queryFn` forwards into the fetch MUST appear in the `queryKey`.** (Query-machinery identifiers — `signal`, `pageParam` — are exempt; they aren't fetch-scoping args.) If the fetch is scoped by `workspaceId`, `cursor`, `limit`, an org id, etc., those values must be part of the key — otherwise distinct fetch args share one cache entry (a cross-tenant / per-param cache collision). The lone exception is a globally-unique id used as the key while a second fetch arg is only an authz scope that cannot collide; annotate those with `// rq-lint-allow: <reason>`. Enforced by the `key-fetch-arg-drift` check in `scripts/check-react-query-patterns.ts`.
29+
2830
## File Structure
2931

3032
```typescript
@@ -142,4 +144,4 @@ const handler = useCallback(() => {
142144

143145
## Enforcement
144146

145-
`scripts/check-react-query-patterns.ts` (`bun run check:react-query`, run in CI) statically enforces these conventions: every `useQuery`/`useInfiniteQuery`/`useSuspenseQuery` declares an explicit `staleTime`, inline `queryFn`s destructure `signal`, `queryKey`s reference a colocated factory rather than an inline literal, and every `*Keys` factory in `hooks/queries/**` exposes an `all` root key. `hooks/queries/**` is a zero-tolerance zone; the rest of `apps/sim/**` is ratcheted against `scripts/check-react-query-patterns.baseline.json`. For a genuine exception, put `// rq-lint-allow: <reason>` on the line directly above the flagged construct.
147+
`scripts/check-react-query-patterns.ts` (`bun run check:react-query`, run in CI) statically enforces these conventions: every `useQuery`/`useInfiniteQuery`/`useSuspenseQuery` declares an explicit `staleTime`, inline `queryFn`s destructure `signal`, `queryKey`s reference a colocated factory rather than an inline literal, every `*Keys` factory in `hooks/queries/**` exposes an `all` root key, and every identifier the `queryFn` forwards into the fetch also appears in the `queryKey` (`key-fetch-arg-drift`). `hooks/queries/**` is a zero-tolerance zone; the rest of `apps/sim/**` is ratcheted against `scripts/check-react-query-patterns.baseline.json`. For a genuine exception, put `// rq-lint-allow: <reason>` on the line directly above the flagged construct.

.claude/rules/sim-stores.md

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ export const useFeatureStore = create<FeatureState>()(
5757

5858
1. Use `devtools` middleware (named stores)
5959
2. Use `persist` only when data should survive reload
60-
3. `partialize` to persist only necessary state
60+
3. `persist` MUST use `partialize` with an explicit whitelist of the durable fields. Exclude transient flags (`isResizing`, drag/hover state) and `_hasHydrated` from the whitelist, and never spread the whole state (`{ ...state }`) — it leaks actions and transient state into storage
6161
4. `_hasHydrated` pattern for persisted stores needing hydration tracking
6262
5. Immutable updates only
6363
6. `set((state) => ...)` when depending on previous state

apps/sim/app/(auth)/verify/use-verification.ts

Lines changed: 38 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,15 @@ import { validateCallbackUrl } from '@/lib/core/security/input-validation'
99

1010
const logger = createLogger('useVerification')
1111

12+
/**
13+
* Mutually-exclusive phases of the email-OTP verification machine.
14+
* - `idle`: awaiting input
15+
* - `verifying`: a verify request is in flight
16+
* - `verified`: code accepted, redirecting
17+
* - `error`: last verify attempt failed (paired with `errorMessage`)
18+
*/
19+
type VerificationStatus = 'idle' | 'verifying' | 'verified' | 'error'
20+
1221
interface UseVerificationParams {
1322
hasEmailService: boolean
1423
isProduction: boolean
@@ -18,9 +27,8 @@ interface UseVerificationParams {
1827
interface UseVerificationReturn {
1928
otp: string
2029
email: string
21-
isLoading: boolean
22-
isVerified: boolean
23-
isInvalidOtp: boolean
30+
status: VerificationStatus
31+
isResending: boolean
2432
errorMessage: string
2533
isOtpComplete: boolean
2634
hasEmailService: boolean
@@ -41,10 +49,9 @@ export function useVerification({
4149
const { refetch: refetchSession } = useSession()
4250
const [otp, setOtp] = useState('')
4351
const [email, setEmail] = useState('')
44-
const [isLoading, setIsLoading] = useState(false)
45-
const [isVerified, setIsVerified] = useState(false)
52+
const [status, setStatus] = useState<VerificationStatus>('idle')
53+
const [isResending, setIsResending] = useState(false)
4654
const [isSendingInitialOtp, setIsSendingInitialOtp] = useState(false)
47-
const [isInvalidOtp, setIsInvalidOtp] = useState(false)
4855
const [errorMessage, setErrorMessage] = useState('')
4956
const [redirectUrl, setRedirectUrl] = useState<string | null>(null)
5057
const [isInviteFlow, setIsInviteFlow] = useState(false)
@@ -96,8 +103,7 @@ export function useVerification({
96103
async function verifyCode() {
97104
if (!isOtpComplete || !email) return
98105

99-
setIsLoading(true)
100-
setIsInvalidOtp(false)
106+
setStatus('verifying')
101107
setErrorMessage('')
102108

103109
try {
@@ -108,7 +114,7 @@ export function useVerification({
108114
})
109115

110116
if (response && !response.error) {
111-
setIsVerified(true)
117+
setStatus('verified')
112118

113119
try {
114120
await refetchSession()
@@ -135,12 +141,9 @@ export function useVerification({
135141
} else {
136142
logger.info('Setting invalid OTP state - API error response')
137143
const message = 'Invalid verification code. Please check and try again.'
138-
setIsInvalidOtp(true)
144+
setStatus('error')
139145
setErrorMessage(message)
140-
logger.info('Error state after API error:', {
141-
isInvalidOtp: true,
142-
errorMessage: message,
143-
})
146+
logger.info('Error state after API error:', { errorMessage: message })
144147
setOtp('')
145148
}
146149
} catch (error: any) {
@@ -155,23 +158,18 @@ export function useVerification({
155158
message = 'Too many failed attempts. Please request a new code.'
156159
}
157160

158-
setIsInvalidOtp(true)
161+
setStatus('error')
159162
setErrorMessage(message)
160-
logger.info('Error state after caught error:', {
161-
isInvalidOtp: true,
162-
errorMessage: message,
163-
})
163+
logger.info('Error state after caught error:', { errorMessage: message })
164164

165165
setOtp('')
166-
} finally {
167-
setIsLoading(false)
168166
}
169167
}
170168

171169
function resendCode() {
172170
if (!email || !hasEmailService || !isEmailVerificationEnabled) return
173171

174-
setIsLoading(true)
172+
setIsResending(true)
175173
setErrorMessage('')
176174

177175
const normalizedEmail = normalizeEmail(email)
@@ -185,32 +183,43 @@ export function useVerification({
185183
setErrorMessage('Failed to resend verification code. Please try again later.')
186184
})
187185
.finally(() => {
188-
setIsLoading(false)
186+
setIsResending(false)
189187
})
190188
}
191189

190+
/**
191+
* On a complete (6-char) code, clear any lingering message — including a
192+
* resend failure (which sets `errorMessage` while `status` stays `idle`) — and
193+
* exit the error state, matching the prior unconditional reset on a full OTP.
194+
*/
192195
function handleOtpChange(value: string) {
193196
if (value.length === 6) {
194-
setIsInvalidOtp(false)
197+
if (status === 'error') setStatus('idle')
195198
setErrorMessage('')
196199
}
197200
setOtp(value)
198201
}
199202

200203
useEffect(() => {
201-
if (otp.length === 6 && email && !isLoading && !isVerified) {
204+
if (
205+
otp.length === 6 &&
206+
email &&
207+
status !== 'verifying' &&
208+
status !== 'verified' &&
209+
!isResending
210+
) {
202211
const timeoutId = setTimeout(() => {
203212
verifyCode()
204213
}, 300)
205214

206215
return () => clearTimeout(timeoutId)
207216
}
208-
}, [otp, email, isLoading, isVerified])
217+
}, [otp, email, status, isResending])
209218

210219
useEffect(() => {
211220
if (typeof window !== 'undefined') {
212221
if (!isEmailVerificationEnabled) {
213-
setIsVerified(true)
222+
setStatus('verified')
214223

215224
const handleRedirect = async () => {
216225
try {
@@ -234,9 +243,8 @@ export function useVerification({
234243
return {
235244
otp,
236245
email,
237-
isLoading,
238-
isVerified,
239-
isInvalidOtp,
246+
status,
247+
isResending,
240248
errorMessage,
241249
isOtpComplete,
242250
hasEmailService,

apps/sim/app/(auth)/verify/verify-content.tsx

Lines changed: 10 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,16 +25,19 @@ function VerificationForm({
2525
const {
2626
otp,
2727
email,
28-
isLoading,
29-
isVerified,
30-
isInvalidOtp,
28+
status,
29+
isResending,
3130
errorMessage,
3231
isOtpComplete,
3332
verifyCode,
3433
resendCode,
3534
handleOtpChange,
3635
} = useVerification({ hasEmailService, isProduction, isEmailVerificationEnabled })
3736

37+
const isVerified = status === 'verified'
38+
const isInvalidOtp = status === 'error'
39+
const isBusy = status === 'verifying' || isResending
40+
3841
const [countdown, setCountdown] = useState(0)
3942
const [isResendDisabled, setIsResendDisabled] = useState(false)
4043

@@ -88,7 +91,7 @@ function VerificationForm({
8891
maxLength={6}
8992
value={otp}
9093
onChange={handleOtpChange}
91-
disabled={isLoading}
94+
disabled={isBusy}
9295
className={cn('gap-2', isInvalidOtp && 'otp-error')}
9396
>
9497
<InputOTPGroup>
@@ -112,10 +115,10 @@ function VerificationForm({
112115

113116
<button
114117
onClick={verifyCode}
115-
disabled={!isOtpComplete || isLoading}
118+
disabled={!isOtpComplete || isBusy}
116119
className={AUTH_SUBMIT_BTN}
117120
>
118-
{isLoading ? (
121+
{isBusy ? (
119122
<span className='flex items-center gap-2'>
120123
<Loader className='size-4' animate />
121124
Verifying…
@@ -138,7 +141,7 @@ function VerificationForm({
138141
<button
139142
className='font-medium text-[var(--landing-text)] underline-offset-4 transition hover:text-white hover:underline'
140143
onClick={handleResend}
141-
disabled={isLoading || isResendDisabled}
144+
disabled={isBusy || isResendDisabled}
142145
>
143146
Resend
144147
</button>

apps/sim/app/(landing)/components/contact/contact-form.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,7 +69,6 @@ export function ContactForm() {
6969
captureClientEvent('landing_contact_submitted', { topic: variables.topic })
7070
setForm(INITIAL_FORM_STATE)
7171
setErrors({})
72-
setSubmitSuccess(true)
7372
},
7473
onError: () => {
7574
turnstileRef.current?.reset()
@@ -78,7 +77,6 @@ export function ContactForm() {
7877

7978
const [form, setForm] = useState<ContactFormState>(INITIAL_FORM_STATE)
8079
const [errors, setErrors] = useState<ContactErrors>({})
81-
const [submitSuccess, setSubmitSuccess] = useState(false)
8280
const [isSubmitting, setIsSubmitting] = useState(false)
8381
const [website, setWebsite] = useState('')
8482
const [widgetReady, setWidgetReady] = useState(false)
@@ -141,6 +139,7 @@ export function ContactForm() {
141139
}
142140

143141
const isBusy = contactMutation.isPending || isSubmitting
142+
const submitSuccess = contactMutation.isSuccess
144143

145144
const submitError = contactMutation.isError
146145
? toError(contactMutation.error).message || 'Failed to send message. Please try again.'
@@ -161,7 +160,7 @@ export function ContactForm() {
161160
</p>
162161
<button
163162
type='button'
164-
onClick={() => setSubmitSuccess(false)}
163+
onClick={() => contactMutation.reset()}
165164
className='mt-6 font-season text-[13px] text-[var(--landing-text)] underline underline-offset-2 transition-opacity hover:opacity-80'
166165
>
167166
Send another message

apps/sim/app/(landing)/components/demo-request/demo-request-modal.tsx

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,22 +69,21 @@ export function DemoRequestModal({ children, theme = 'dark' }: DemoRequestModalP
6969
const [open, setOpen] = useState(false)
7070
const [form, setForm] = useState<DemoRequestFormState>(INITIAL_FORM_STATE)
7171
const [errors, setErrors] = useState<DemoRequestErrors>({})
72-
const [submitSuccess, setSubmitSuccess] = useState(false)
7372

7473
const demoMutation = useMutation({
7574
mutationFn: submitDemoRequest,
7675
onSuccess: (_data, variables) => {
7776
captureClientEvent('landing_demo_request_submitted', {
7877
company_size: variables.companySize,
7978
})
80-
setSubmitSuccess(true)
8179
},
8280
})
8381

82+
const submitSuccess = demoMutation.isSuccess
83+
8484
function resetForm() {
8585
setForm(INITIAL_FORM_STATE)
8686
setErrors({})
87-
setSubmitSuccess(false)
8887
demoMutation.reset()
8988
}
9089

apps/sim/app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry/resource-registry.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -255,9 +255,9 @@ const RESOURCE_INVALIDATORS: Record<
255255
scheduledtask: (qc, wId) => {
256256
qc.invalidateQueries({ queryKey: scheduleKeys.list(wId) })
257257
},
258-
log: (qc, _wId, id) => {
258+
log: (qc, wId, id) => {
259259
qc.invalidateQueries({ queryKey: logKeys.details() })
260-
qc.invalidateQueries({ queryKey: logKeys.detail(id) })
260+
qc.invalidateQueries({ queryKey: logKeys.detail(wId, id) })
261261
},
262262
/**
263263
* Integrations are sourced from the static integration catalog

apps/sim/app/workspace/[workspaceId]/knowledge/components/edit-knowledge-base-modal/edit-knowledge-base-modal.tsx

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

3-
import { memo, useEffect, useState } from 'react'
3+
import { memo, useRef, useState } from 'react'
44
import { createLogger } from '@sim/logger'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import {
@@ -44,15 +44,21 @@ export const EditKnowledgeBaseModal = memo(function EditKnowledgeBaseModal({
4444
const [isSubmitting, setIsSubmitting] = useState(false)
4545
const [error, setError] = useState<string | null>(null)
4646

47-
useEffect(() => {
47+
/**
48+
* Seed the fields only on the closed → open transition (render-phase reset),
49+
* so a prop change while the modal is open never clobbers in-progress edits.
50+
*/
51+
const prevOpenRef = useRef(open)
52+
if (prevOpenRef.current !== open) {
53+
prevOpenRef.current = open
4854
if (open) {
4955
setName(initialName)
5056
setDescription(initialDescription)
5157
setNameError(null)
5258
setDescriptionError(null)
5359
setError(null)
5460
}
55-
}, [open, initialName, initialDescription])
61+
}
5662

5763
const validate = (): boolean => {
5864
let valid = true

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -505,7 +505,7 @@ export default function Logs() {
505505
}
506506
}, [contextMenuLog])
507507

508-
const cancelExecution = useCancelExecution()
508+
const cancelExecution = useCancelExecution(workspaceId)
509509
const retryExecution = useRetryExecution()
510510

511511
const handleCancelExecution = useCallback(() => {
@@ -525,7 +525,7 @@ export default function Logs() {
525525

526526
try {
527527
const detailLog = await queryClient.fetchQuery({
528-
queryKey: logKeys.detail(logId),
528+
queryKey: logKeys.detail(workspaceId, logId),
529529
queryFn: ({ signal }) => fetchLogDetail(logId, workspaceId, signal),
530530
staleTime: 30 * 1000,
531531
})

0 commit comments

Comments
 (0)