Skip to content

Commit 78f7955

Browse files
committed
fix(chat): bound preview work and preserve readable tables
1 parent dfcb4a9 commit 78f7955

15 files changed

Lines changed: 279 additions & 27 deletions

File tree

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,50 @@
1+
/** @vitest-environment node */
2+
import { NextRequest } from 'next/server'
3+
import { beforeEach, describe, expect, it, vi } from 'vitest'
4+
5+
const mocks = vi.hoisted(() => ({
6+
fetch: vi.fn(),
7+
get: vi.fn(),
8+
set: vi.fn(),
9+
}))
10+
vi.mock('@/lib/auth', () => ({ getSession: async () => ({ user: { id: 'test-user' } }) }))
11+
vi.mock('@/lib/core/rate-limiter/route-helpers', () => ({
12+
enforceUserRateLimit: async () => null,
13+
}))
14+
vi.mock('@/lib/api/server', () => ({
15+
parseRequest: async () => ({
16+
success: true,
17+
data: { query: { url: 'https://example.com/guide' } },
18+
}),
19+
}))
20+
vi.mock('@/lib/core/config/redis', () => ({ getRedisClient: () => mocks }))
21+
vi.mock('@/lib/core/network/context.server', () => ({
22+
runWithOutboundOrganization: (_organization: null, run: () => unknown) => run(),
23+
}))
24+
vi.mock('@/lib/core/utils/with-route-handler', () => ({
25+
withRouteHandler: (handler: unknown) => handler,
26+
}))
27+
vi.mock('@/lib/link-preview/fetch-preview', () => ({ fetchLinkPreview: mocks.fetch }))
28+
29+
import { GET } from '@/app/api/link-preview/route'
30+
31+
const complete = { title: 'Guide', description: null, siteName: null }
32+
33+
describe('link preview cache lifetime', () => {
34+
beforeEach(() => {
35+
vi.clearAllMocks()
36+
mocks.get.mockResolvedValue(null)
37+
mocks.set.mockResolvedValue('OK')
38+
})
39+
40+
it.each([
41+
{ preview: { ...complete, imageRetryable: true }, ttl: 60 },
42+
{ preview: complete, ttl: 24 * 60 * 60 },
43+
{ preview: null, ttl: 60 * 60 },
44+
])('caches $preview for $ttl seconds', async ({ preview, ttl }) => {
45+
mocks.fetch.mockResolvedValue(preview)
46+
const response = await GET(new NextRequest('https://example.com/api/link-preview'))
47+
expect(await response.json()).toEqual({ preview })
48+
expect(mocks.set).toHaveBeenCalledWith(expect.any(String), JSON.stringify(preview), 'EX', ttl)
49+
})
50+
})

‎apps/sim/app/api/link-preview/route.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ const logger = createLogger('LinkPreviewAPI')
1717

1818
const CACHE_TTL_SECONDS = 24 * 60 * 60
1919
const NEGATIVE_CACHE_TTL_SECONDS = 60 * 60
20+
const RETRYABLE_CACHE_TTL_SECONDS = 60
2021
const CACHE_KEY_PREFIX = 'link-preview:v2:'
2122

2223
export const GET = withRouteHandler(async (request: NextRequest) => {
@@ -62,7 +63,11 @@ export const GET = withRouteHandler(async (request: NextRequest) => {
6263
}
6364

6465
if (redis) {
65-
const ttl = preview ? CACHE_TTL_SECONDS : NEGATIVE_CACHE_TTL_SECONDS
66+
const ttl = preview?.imageRetryable
67+
? RETRYABLE_CACHE_TTL_SECONDS
68+
: preview
69+
? CACHE_TTL_SECONDS
70+
: NEGATIVE_CACHE_TTL_SECONDS
6671
try {
6772
await redis.set(cacheKey, JSON.stringify(preview), 'EX', ttl)
6873
} catch (error) {

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/editor-lifecycle.test.tsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ vi.mock('next/navigation', () => ({
3131
usePathname: () => '/workspace/workspace-1/files',
3232
useRouter: () => ({ push: vi.fn() }),
3333
}))
34+
vi.mock('@/app/_styles/fonts/inter/inter', () => ({ inter: { variable: 'test-inter-variable' } }))
3435
vi.mock('@/lib/auth/auth-client', () => ({ useSession: () => ({ data: null, isPending: false }) }))
3536
vi.mock('@/hooks/queries/workspace-files', () => ({
3637
useUploadWorkspaceFile: () => ({ mutateAsync: uploadFile }),

‎apps/sim/app/workspace/[workspaceId]/files/components/file-viewer/rich-markdown-editor/rich-markdown-editor.tsx‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ import {
2525
} from '@/lib/mothership/chat/selection-context'
2626
import type { FileDownloadSource } from '@/lib/uploads/client/download'
2727
import type { WorkspaceFileRecord } from '@/lib/uploads/contexts/workspace'
28+
import { inter } from '@/app/_styles/fonts/inter/inter'
2829
import { FindBar } from '@/app/workspace/[workspaceId]/components/find-bar/find-bar'
2930
import { FileSaveConflict } from '@/app/workspace/[workspaceId]/files/components/file-viewer/file-save-conflict'
3031
import { PreviewLoadingFrame } from '@/app/workspace/[workspaceId]/files/components/file-viewer/preview-shared'
@@ -118,8 +119,10 @@ function warnRichMarkdownPasteLimit(reason?: 'paste' | 'formatting') {
118119
* {@link ReadOnlyPlaceholder} render into, so the two are geometrically identical and the placeholder →
119120
* live swap never reflows. Shared as one constant to keep them in lockstep.
120121
*/
121-
const EDITOR_SURFACE_CLASS =
122-
'mx-auto flex w-full max-w-[48rem] flex-1 flex-col px-8 py-6 selection:bg-[var(--selection-bg)] selection:text-[var(--text-primary)] dark:selection:bg-[var(--selection-dark)] dark:selection:text-white'
122+
const EDITOR_SURFACE_CLASS = cn(
123+
'mx-auto flex w-full max-w-[48rem] flex-1 flex-col px-8 py-6 selection:bg-[var(--selection-bg)] selection:text-[var(--text-primary)] dark:selection:bg-[var(--selection-dark)] dark:selection:text-white',
124+
inter.variable
125+
)
123126

124127
/** ProseMirror block positions do not correspond to markdown source line numbers. */
125128
function buildEditorSelectionContext(

‎apps/sim/app/workspace/[workspaceId]/home/components/message-content/components/chat-content/chat-content.tsx‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -282,20 +282,29 @@ interface TableCellContentProps extends ExtraProps {
282282

283283
function TableCellContent({ children, node }: TableCellContentProps) {
284284
if (hasInteractiveTableContent(node)) {
285-
return <div className='whitespace-normal [overflow-wrap:anywhere]'>{children}</div>
285+
return (
286+
<div className='min-w-[160px] max-w-[320px] whitespace-normal [overflow-wrap:anywhere]'>
287+
{children}
288+
</div>
289+
)
286290
}
287291
return (
288-
<OverflowText label={extractTextContent(children)} className='[&_code]:whitespace-nowrap'>
289-
{children}
290-
</OverflowText>
292+
<span className='inline-block max-w-full align-middle'>
293+
<OverflowText
294+
label={extractTextContent(children)}
295+
className='[&_code]:whitespace-nowrap [@media(hover:hover)]:max-w-[320px]'
296+
>
297+
{children}
298+
</OverflowText>
299+
</span>
291300
)
292301
}
293302

294303
const MARKDOWN_COMPONENTS = {
295304
table({ children }: { children?: React.ReactNode }) {
296305
return (
297306
<div className='not-prose my-4 w-full overflow-x-auto [&_strong]:font-semibold'>
298-
<table className='w-full table-fixed border-collapse [&_tbody_tr:last-child_td]:border-b-0'>
307+
<table className='min-w-full table-auto border-collapse [&_tbody_tr:last-child_td]:border-b-0'>
299308
{children}
300309
</table>
301310
</div>
Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
/** @vitest-environment jsdom */
2+
import { act } from 'react'
3+
import { QueryClient, QueryClientProvider } from '@tanstack/react-query'
4+
import { createRoot, type Root } from 'react-dom/client'
5+
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
6+
7+
const { requestMock } = vi.hoisted(() => ({ requestMock: vi.fn() }))
8+
vi.mock('@/lib/api/client/request', () => ({ requestJson: requestMock }))
9+
10+
import { linkPreviewKeys, useLinkPreview } from '@/hooks/queries/link-preview'
11+
12+
const URL = 'https://example.com/guide'
13+
const complete = { title: 'Guide', description: null, siteName: null }
14+
let client: QueryClient
15+
let container: HTMLDivElement
16+
let root: Root
17+
18+
function Preview() {
19+
useLinkPreview(URL)
20+
return null
21+
}
22+
23+
beforeEach(() => {
24+
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
25+
requestMock.mockReset().mockResolvedValue({ preview: complete })
26+
client = new QueryClient({ defaultOptions: { queries: { retry: false } } })
27+
container = document.createElement('div')
28+
document.body.appendChild(container)
29+
root = createRoot(container)
30+
})
31+
afterEach(() => {
32+
act(() => root.unmount())
33+
container.remove()
34+
client.clear()
35+
})
36+
37+
describe('link preview freshness', () => {
38+
it.each([true, false])(
39+
'retries a one-minute-old preview only when its image is retryable: %s',
40+
async (retryable) => {
41+
client.setQueryData(
42+
linkPreviewKeys.detail(URL),
43+
{ preview: { ...complete, ...(retryable ? { imageRetryable: true } : {}) } },
44+
{ updatedAt: Date.now() - 61_000 }
45+
)
46+
await act(async () => {
47+
root.render(
48+
<QueryClientProvider client={client}>
49+
<Preview />
50+
</QueryClientProvider>
51+
)
52+
})
53+
expect(requestMock).toHaveBeenCalledTimes(retryable ? 1 : 0)
54+
if (retryable) {
55+
expect(requestMock).toHaveBeenCalledWith(expect.anything(), {
56+
query: { url: URL },
57+
signal: expect.any(AbortSignal),
58+
})
59+
}
60+
}
61+
)
62+
})

‎apps/sim/hooks/queries/link-preview.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { getLinkPreviewContract, type LinkPreviewResponse } from '@/lib/api/cont
44

55
/** Previews are near-immutable page metadata; the server also caches for 24h. */
66
export const LINK_PREVIEW_STALE_TIME = 60 * 60 * 1000
7+
export const LINK_PREVIEW_RETRY_STALE_TIME = 60 * 1000
78

89
export const linkPreviewKeys = {
910
all: ['link-preview'] as const,
@@ -18,15 +19,18 @@ async function fetchLinkPreview(url: string, signal?: AbortSignal): Promise<Link
1819
/**
1920
* OG metadata for an external URL, fetched through the SSRF-hardened
2021
* `/api/link-preview` proxy. Mounted by the source preview on hover or focus; results are long-lived
21-
* (client staleTime + 24h server-side Redis cache) and failures are not
22-
* retried.
22+
* (client staleTime + 24h server-side Redis cache). A deferred thumbnail becomes stale after
23+
* one minute so a later hover can retry; there is no background polling.
2324
*/
2425
export function useLinkPreview(url?: string) {
2526
return useQuery({
2627
queryKey: linkPreviewKeys.detail(url),
2728
queryFn: ({ signal }) => fetchLinkPreview(url as string, signal),
2829
enabled: Boolean(url),
29-
staleTime: LINK_PREVIEW_STALE_TIME,
30+
staleTime: (query) =>
31+
query.state.data?.preview?.imageRetryable
32+
? LINK_PREVIEW_RETRY_STALE_TIME
33+
: LINK_PREVIEW_STALE_TIME,
3034
retry: false,
3135
})
3236
}

‎apps/sim/lib/api/contracts/link-preview.ts‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,12 @@ export const linkPreviewResponseSchema = z.object({
2121
.max(180_000)
2222
.regex(/^data:image\/webp;base64,[A-Za-z0-9+/=]+$/)
2323
.optional(),
24+
imageRetryable: z
25+
.literal(true)
26+
.describe(
27+
'The optional image was deferred or temporarily unavailable; retry on later intent.'
28+
)
29+
.optional(),
2430
})
2531
.nullable(),
2632
})

‎apps/sim/lib/core/security/egress/validate.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ export type EgressValidationResult = EgressValidationSuccess | EgressValidationF
4545
export interface EgressValidationOptions {
4646
/** Omit destination-derived values from logs when the URL contains protected context. */
4747
logDetails?: boolean
48+
/** Cancels DNS validation before a guarded connection can begin. */
49+
signal?: AbortSignal
4850
}
4951

5052
type EgressDenial = Extract<EgressDecision, { allowed: false }>
@@ -77,6 +79,7 @@ export async function validateEgressUrl(
7779
profile: EgressProfile,
7880
options: EgressValidationOptions = {}
7981
): Promise<EgressValidationResult> {
82+
options.signal?.throwIfAborted()
8083
if (!url || typeof url !== 'string') {
8184
return { isValid: false, error: `${paramName} is required and must be a string` }
8285
}
@@ -113,8 +116,9 @@ export async function validateEgressUrl(
113116

114117
let addresses: string[]
115118
try {
116-
addresses = (await resolveHostAddresses(host)).addresses
119+
addresses = (await resolveHostAddresses(host, { signal: options.signal })).addresses
117120
} catch (error) {
121+
options.signal?.throwIfAborted()
118122
logger.warn(
119123
'DNS lookup failed',
120124
options.logDetails === false

‎apps/sim/lib/core/security/input-validation.server.test.ts‎

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,10 @@ vi.mock('@/lib/core/config/env-flags', () => ({
2626
getProxyUrl: () => undefined,
2727
}))
2828

29-
import { validateUrlWithDNS } from '@/lib/core/security/input-validation.server'
29+
import {
30+
secureFetchWithValidation,
31+
validateUrlWithDNS,
32+
} from '@/lib/core/security/input-validation.server'
3033

3134
/**
3235
* Shapes a resolver answer the way `resolveHostAddresses` does, including its
@@ -141,4 +144,25 @@ describe('validateUrlWithDNS address classification', () => {
141144
})
142145
expect(JSON.stringify(mockWarn.mock.calls)).not.toContain('credential-host-canary')
143146
})
147+
148+
it('forwards fetch cancellation through DNS preflight without treating it as a resolver failure', async () => {
149+
const controller = new AbortController()
150+
mockResolve.mockImplementationOnce(
151+
(_host, options) =>
152+
new Promise((_, reject) => {
153+
options.signal.addEventListener('abort', () => reject(options.signal.reason), {
154+
once: true,
155+
})
156+
})
157+
)
158+
const pending = secureFetchWithValidation('https://example.com/preview', {
159+
profile: 'contentFetch',
160+
signal: controller.signal,
161+
})
162+
const rejection = expect(pending).rejects.toThrow('Preview deadline')
163+
controller.abort(new Error('Preview deadline'))
164+
await rejection
165+
expect(mockResolve).toHaveBeenCalledWith('example.com', { signal: controller.signal })
166+
expect(mockWarn).not.toHaveBeenCalled()
167+
})
144168
})

0 commit comments

Comments
 (0)