Skip to content

Commit b314160

Browse files
icecrasher321claude
andcommitted
fix(desktop): review fixes — OAuth error handling, query freshness, invitations
Findings from an end-to-end review of the desktop work, fixed and verified. OAuth connect/login handoff: - Add a friendly /oauth-error landing page + onAPIError.errorURL so provider Cancel/Deny (which Better Auth redirects before the flow state is parsed) no longer dead-ends on a 404; re-initiating supersedes the idle loopback. - Stop a post-consent failure from reporting success (drop the baked-in errorCallbackURL param that collided with Better Auth's appended code; coerce an array error defensively on the complete page). - Guard the desktop connect listener with the same context-age check the web routers use, so an abandoned flow can't mislabel a later completion. - Clear an orphaned pending handoff when a loopback re-bind fails. Query freshness (desktop refetchOnWindowFocus): - Pin refetchOnWindowFocus off on queries that seed editable forms (environment/secrets, credential detail, schedules) so a background focus refetch can't drop an unsaved draft, and on the useWorkflowStates fan-out so returning to a large table doesn't fire N heavy envelope fetches. All no-ops on web (default already false). Invitations (in-app pending invitations): - Map accept/decline failures to friendly copy instead of raw machine codes. - Invalidate subscription + refresh session on accept (parity with the email path); reconcile the list on failure (onSettled) so dead rows drop. - Gate the modal's query on open so it no longer fetches on every app load. CI: - Wrap the latest-mac.yml update-feed route in withRouteHandler and allowlist it as a non-boundary route (input-less, YAML) so the contract audit passes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
1 parent 4d7cde9 commit b314160

18 files changed

Lines changed: 216 additions & 24 deletions

File tree

apps/desktop/src/main/handoff.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,8 +177,12 @@ export function createHandoffManager(
177177
params: Record<string, string>
178178
): Promise<boolean> => {
179179
const state = generateShortId(STATE_LENGTH)
180+
// startLoopback() already tore down any prior server; if this bind fails,
181+
// clear the now-orphaned pending so a superseded flow can't linger as a
182+
// dangling entry pointing at a server that no longer exists.
180183
const port = await startLoopback()
181184
if (!port) {
185+
clear()
182186
return false
183187
}
184188
pending = { state, createdAt: now(), kind }
Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,26 @@
1+
/**
2+
* @vitest-environment jsdom
3+
*/
4+
import type { SimDesktopApi } from '@sim/desktop-bridge'
5+
import { afterEach, describe, expect, it } from 'vitest'
6+
import { makeQueryClient } from '@/app/_shell/providers/get-query-client'
7+
8+
function focusRefetchDefault(): boolean | 'always' | ((...args: unknown[]) => boolean) | undefined {
9+
return makeQueryClient().getDefaultOptions().queries?.refetchOnWindowFocus
10+
}
11+
12+
describe('makeQueryClient refetchOnWindowFocus default', () => {
13+
afterEach(() => {
14+
// biome-ignore lint/performance/noDelete: test teardown of a global stub
15+
delete (window as { simDesktop?: SimDesktopApi }).simDesktop
16+
})
17+
18+
it('is off on the web (no desktop bridge) — tab-switch focus is noisy there', () => {
19+
expect(focusRefetchDefault()).toBe(false)
20+
})
21+
22+
it('is on in the desktop app — refetches stale queries when the long-lived window refocuses', () => {
23+
;(window as { simDesktop?: SimDesktopApi }).simDesktop = {} as SimDesktopApi
24+
expect(focusRefetchDefault()).toBe(true)
25+
})
26+
})

apps/sim/app/_shell/providers/get-query-client.ts

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,23 @@
11
import { defaultShouldDehydrateQuery, isServer, QueryClient } from '@tanstack/react-query'
2+
import { isDesktopApp } from '@/lib/desktop'
23

3-
function makeQueryClient() {
4+
export function makeQueryClient() {
45
return new QueryClient({
56
defaultOptions: {
67
queries: {
78
staleTime: 30 * 1000,
89
gcTime: 5 * 60 * 1000,
9-
refetchOnWindowFocus: false,
10+
// The desktop app window lives for days, so cross-session changes —
11+
// an admin upgrading your org/workspace role, a workspace you were
12+
// auto-added to, seat/entitlement changes — would otherwise stay
13+
// stale until a manual reload (there is no push channel for them).
14+
// Refetch stale queries when the app window regains focus: the "user
15+
// came back after doing something elsewhere" signal, which is exactly
16+
// this bug class. staleTime still gates it, so rapid focus changes
17+
// cost nothing. Left off on the web, where tab-switch focus events are
18+
// frequent and noisy. Per-query overrides (e.g. useWorkspaceSchedules
19+
// pins this off) always win over this default.
20+
refetchOnWindowFocus: isDesktopApp(),
1021
retry: 1,
1122
retryOnMount: false,
1223
},

apps/sim/app/api/desktop/update/latest-mac.yml/route.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import { createLogger } from '@sim/logger'
22
import { NextResponse } from 'next/server'
33
import { env } from '@/lib/core/config/env'
4+
import { withRouteHandler } from '@/lib/core/utils/with-route-handler'
45
import {
56
channelForHostname,
67
DESKTOP_RELEASE_REPO,
@@ -28,7 +29,7 @@ const RELEASES_API_URL = `https://api.github.com/repos/${DESKTOP_RELEASE_REPO}/r
2829
* design: the updater's HTTP client carries no session, and the response
2930
* only describes public GitHub release artifacts.
3031
*/
31-
export async function GET(): Promise<Response> {
32+
export const GET = withRouteHandler(async (): Promise<Response> => {
3233
const hostname = new URL(env.NEXT_PUBLIC_APP_URL).hostname
3334
const channel = channelForHostname(hostname)
3435

@@ -89,4 +90,4 @@ export async function GET(): Promise<Response> {
8990
'cache-control': `public, max-age=${REVALIDATE_SECONDS}`,
9091
},
9192
})
92-
}
93+
})

apps/sim/app/desktop/connect/complete/page.tsx

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@ export default async function ConnectCompletePage({ searchParams }: ConnectCompl
4343
return <InvalidRequest />
4444
}
4545

46-
const error = sanitizeOAuthErrorSlug(params.error)
46+
// Defensive: if `error` somehow arrives as a repeated query key (array), a
47+
// failure must never read as success — take the first code.
48+
const rawError = Array.isArray(params.error) ? params.error[0] : params.error
49+
const error = sanitizeOAuthErrorSlug(rawError)
4750
redirect(buildConnectLoopbackUrl(state, port, error ?? undefined))
4851
}

apps/sim/app/desktop/connect/connect-launcher.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -27,9 +27,12 @@ export function ConnectLauncher({ providerId, completePath }: ConnectLauncherPro
2727
await client.oauth2.link({
2828
providerId,
2929
callbackURL: completePath,
30-
// Failed flows still bounce to the loopback so the desktop app can
31-
// surface the failure instead of waiting out the handoff TTL.
32-
errorCallbackURL: `${completePath}&error=oauth_failed`,
30+
// Failed flows bounce to the same complete page (which forwards the
31+
// failure to the loopback) instead of waiting out the handoff TTL.
32+
// Do NOT bake in a query param here: better-auth appends its own
33+
// `&error=<code>`, and a second `error` key deserializes to an array
34+
// that the complete page can't read — so it would look like success.
35+
errorCallbackURL: completePath,
3336
})
3437
} catch (err) {
3538
setError(getErrorMessage(err, 'Could not start the connection.'))

apps/sim/app/oauth-error/page.tsx

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import type { Metadata } from 'next'
2+
3+
export const metadata: Metadata = {
4+
title: 'Sign-in couldn’t be completed',
5+
robots: { index: false },
6+
}
7+
8+
export const dynamic = 'force-dynamic'
9+
10+
interface OAuthErrorPageProps {
11+
searchParams: Promise<Record<string, string | string[] | undefined>>
12+
}
13+
14+
/**
15+
* Landing page for OAuth flows that end in an error before the flow state can
16+
* be parsed — most commonly the user clicking "Cancel"/"Deny" at the
17+
* provider's consent screen (`?error=access_denied`). Better Auth redirects
18+
* such errors to `onAPIError.errorURL` (this page) BEFORE it can honor a
19+
* per-flow `errorCallbackURL`, so the desktop handoff's loopback is never
20+
* pinged. Without this page those errors 404'd (a dead-end); here the user
21+
* gets a clear message and a way back. Re-initiating the sign-in/connect from
22+
* the app supersedes the idle handoff, so no explicit hand-back is needed.
23+
*/
24+
const FRIENDLY: Record<string, string> = {
25+
access_denied: 'You declined the request at the provider, so nothing was connected.',
26+
oAuth_code_missing: 'The provider didn’t return a valid response. Please try again.',
27+
}
28+
29+
function messageForError(code: string | undefined): string {
30+
if (code && FRIENDLY[code]) return FRIENDLY[code]
31+
return 'The sign-in couldn’t be completed. Please try again.'
32+
}
33+
34+
export default async function OAuthErrorPage({ searchParams }: OAuthErrorPageProps) {
35+
const params = await searchParams
36+
const code = typeof params.error === 'string' ? params.error : undefined
37+
38+
return (
39+
<main className='flex min-h-screen items-center justify-center px-6'>
40+
<div className='max-w-sm text-center'>
41+
<h1 className='font-semibold text-foreground text-lg'>Couldn’t complete that</h1>
42+
<p className='mt-2 text-muted-foreground text-sm'>{messageForError(code)}</p>
43+
<p className='mt-4 text-muted-foreground text-sm'>
44+
You can close this tab and try again from Sim.
45+
</p>
46+
</div>
47+
</main>
48+
)
49+
}

apps/sim/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/pending-invitations/view-invitations-modal.tsx

Lines changed: 11 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import { createLogger } from '@sim/logger'
55
import { getErrorMessage } from '@sim/utils/errors'
66
import { useRouter } from 'next/navigation'
77
import type { InvitationDetails } from '@/lib/api/contracts/invitations'
8+
import { getInvitationErrorMessage } from '@/lib/invitations/error-messages'
89
import {
910
useAcceptMyInvitation,
1011
useDeclineMyInvitation,
@@ -50,7 +51,7 @@ interface ViewInvitationsModalProps {
5051
* the joined workspace; declining keeps it open for the remaining rows.
5152
*/
5253
export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModalProps) {
53-
const { data: invitations } = useMyPendingInvitations()
54+
const { data: invitations } = useMyPendingInvitations(open)
5455
const acceptInvitation = useAcceptMyInvitation()
5556
const declineInvitation = useDeclineMyInvitation()
5657
const router = useRouter()
@@ -65,7 +66,12 @@ export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModa
6566
router.push(result.redirectPath)
6667
} catch (error) {
6768
logger.error('Failed to accept invitation', { error })
68-
toast.error(getErrorMessage(error, 'Could not accept the invitation. It may have expired.'))
69+
toast.error(
70+
getInvitationErrorMessage(
71+
getErrorMessage(error, ''),
72+
'Could not accept the invitation. It may have expired.'
73+
)
74+
)
6975
}
7076
}
7177

@@ -74,7 +80,9 @@ export function ViewInvitationsModal({ open, onOpenChange }: ViewInvitationsModa
7480
await declineInvitation.mutateAsync({ invitationId: inv.id })
7581
} catch (error) {
7682
logger.error('Failed to decline invitation', { error })
77-
toast.error(getErrorMessage(error, 'Could not decline the invitation.'))
83+
toast.error(
84+
getInvitationErrorMessage(getErrorMessage(error, ''), 'Could not decline the invitation.')
85+
)
7886
}
7987
}
8088

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

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import {
3232
import { InviteModal } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/invite-modal'
3333
import { ViewInvitationsMenuItem } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/pending-invitations/view-invitations-menu-item'
3434
import { ViewInvitationsModal } from '@/app/workspace/[workspaceId]/w/components/sidebar/components/workspace-header/components/pending-invitations/view-invitations-modal'
35+
import { invitationKeys } from '@/hooks/queries/invitations'
3536
import {
3637
type Workspace,
3738
type WorkspaceCreationPolicy,
@@ -370,11 +371,12 @@ function WorkspaceHeaderImpl({
370371
return
371372
}
372373
if (open) {
373-
// The workspace list is mounted app-wide, so it never refetches
374-
// on its own (no focus refetch). Opening the switcher is the
375-
// "user is looking" moment: refetch when stale so workspaces
376-
// the user was auto-added to appear without a page refresh.
374+
// Opening the switcher is the "user is looking" moment: refetch
375+
// stale server state so a workspace the user was auto-added to,
376+
// or a fresh pending invitation, appears without a page refresh
377+
// (these are app-wide queries with no focus refetch on the web).
377378
void queryClient.refetchQueries({ queryKey: workspaceKeys.lists(), stale: true })
379+
void queryClient.refetchQueries({ queryKey: invitationKeys.mine(), stale: true })
378380
}
379381
setIsWorkspaceMenuOpen(open)
380382
}}

apps/sim/hooks/queries/credentials.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,10 @@ export function useWorkspaceCredential(credentialId?: string, enabled = true) {
9292
},
9393
enabled: Boolean(credentialId) && enabled,
9494
staleTime: WORKSPACE_CREDENTIAL_DETAIL_STALE_TIME,
95+
// The credential-detail form seeds editable name/description fields from
96+
// this data, so a background focus refetch during an edit could clobber
97+
// an unsaved draft. Off the desktop focus-refetch default; no-op on web.
98+
refetchOnWindowFocus: false,
9599
})
96100
}
97101

0 commit comments

Comments
 (0)