Skip to content

Commit 76c36ff

Browse files
fix(apps/chat): stabilize image attachments and login redirects (#5079)
Co-authored-by: Roland Schlaefli <rolandschlaefli@gmail.com>
1 parent bc31b07 commit 76c36ff

20 files changed

Lines changed: 774 additions & 315 deletions

AGENTS.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -211,9 +211,12 @@ Without Traefik, use `http://localhost:<port>` directly. The `*.klicker.com` dom
211211
- **Participant email uniqueness across auth modes**: Prisma enforces `Participant @@unique([email, isSSOAccount])`, so the same normalized email can exist once as manual and once as SSO. To block new cross-mode duplicates, account creation must explicitly check normalized email collisions in service logic. (`packages/graphql/src/services/accounts.ts`)
212212
- **Helm v3 secrets are external**: `deploy/charts/klicker-uzh-v3/` deployments reference `envFrom.secretRef` names, but the chart currently defines no `Secret` manifests; secrets must be provisioned out-of-band with matching names. (`deploy/charts/klicker-uzh-v3/templates/`)
213213
- **Edited chat image hydration needs a stable source id**: Edited branch messages in `apps/chat` get fresh local message ids, so image hydration must distinguish the local target message id from the persisted source message id (`attachmentSourceMessageId`) when fetching and merging attachments. (`apps/chat/src/hooks/useThreadManagement.ts`, `apps/chat/src/stores/chatStore.ts`)
214+
- **Assistant UI chat drop targets**: `ComposerPrimitive.AttachmentDropzone` must wrap both normal and edit chat composer roots; it owns the drag/drop capture handlers that prevent native browser file navigation, while Klicker-specific limits stay in local composer code. (`apps/chat/src/components/thread.tsx`)
214215
- **Deployed chat model registries default attachment support off**: `apps/chat` loads `CHAT_MODEL_REGISTRY_JSON` via `chatModelRegistry.ts`, where omitted `supportsImageAttachments` values default to `false`; if deployment values override the built-in registry, each image-capable model must set the flag explicitly in `deploy/env-uzh-*/values.yaml` or the attach button disappears.
215216
- **Embedded PWA messaging trust boundary**: For embedded PWA pages, use a parent-initiated `postMessage` handshake to capture `event.origin` and avoid `'*'` target origins; do not add a second per-platform messaging allowlist in page code. Embedding permission remains enforced separately by ingress `frame-ancestors`. (`apps/frontend-pwa/src/pages/course/[courseId]/practiceQuizzes/[id].tsx`, `deploy/charts/klicker-uzh-v3/templates/ingress-frontend-pwa.yaml`)
216217
- **Local embed harness target**: `util/embed-harness/` is for local verification only and should target the branch-local PWA (`http://127.0.0.1:3101/...`), not `https://pwa.klicker.com/...`, because production CSP / `frame-ancestors` blocks localhost embedding. (`util/embed-harness/`)
218+
- **Chat PWA login redirects**: `apps/chat/src/app/noLogin/page.tsx` must pass an absolute chat URL to the PWA login `redirect_to`; a relative chatbot path makes the PWA redirect to its own domain and 404. Local chat dev also needs ignored local env values for the backend `APP_SECRET` and `DATABASE_URL` so participant cookies verify and Prisma can load chatbot data.
219+
- **Chat Vitest alias resolution**: `apps/chat/vitest.config.ts` mirrors the app `@/*` alias from `apps/chat/tsconfig.json`; keep this in sync when adding client tests for modules that import from `@/src/...`.
217220

218221
## Factory Skills (AI Assistance)
219222

apps/chat/src/app/noLogin/page.tsx

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,24 @@ interface NoLoginPageProps {
44
searchParams?: Promise<{ redirectTo?: string | string[] }>
55
}
66

7+
function getChatRedirectUrl(redirectTo: string | undefined) {
8+
if (!redirectTo) return undefined
9+
10+
const chatBaseUrl = process.env.NEXT_PUBLIC_CHAT_URL
11+
? process.env.NEXT_PUBLIC_CHAT_URL.replace(/\/$/, '')
12+
: 'https://chat.klicker.uzh.ch'
13+
14+
try {
15+
const chatUrl = new URL(chatBaseUrl)
16+
const redirectUrl = new URL(redirectTo, chatUrl)
17+
18+
if (redirectUrl.origin !== chatUrl.origin) return undefined
19+
return redirectUrl.toString()
20+
} catch {
21+
return undefined
22+
}
23+
}
24+
725
export default async function Page({ searchParams }: NoLoginPageProps) {
826
const resolvedSearchParams = (await searchParams) ?? {}
927
const redirectToParam = resolvedSearchParams.redirectTo
@@ -14,9 +32,10 @@ export default async function Page({ searchParams }: NoLoginPageProps) {
1432
const loginBaseUrl = process.env.NEXT_PUBLIC_PWA_URL
1533
? process.env.NEXT_PUBLIC_PWA_URL.replace(/\/$/, '')
1634
: 'https://pwa.klicker.uzh.ch'
35+
const redirectUrl = getChatRedirectUrl(redirectTo)
1736

18-
const loginHref = redirectTo
19-
? `${loginBaseUrl}/login?redirect_to=${encodeURIComponent(redirectTo)}`
37+
const loginHref = redirectUrl
38+
? `${loginBaseUrl}/login?redirect_to=${encodeURIComponent(redirectUrl)}`
2039
: `${loginBaseUrl}/login`
2140

2241
return (
@@ -29,10 +48,10 @@ export default async function Page({ searchParams }: NoLoginPageProps) {
2948
You need to create a KlickerUZH account or log in before you can
3049
access this chatbot.
3150
</p>
32-
{redirectTo && (
51+
{redirectUrl && (
3352
<p className="text-muted-foreground mt-2 text-sm">
3453
After logging in, return to{' '}
35-
<span className="font-medium">{redirectTo}</span> to continue your
54+
<span className="font-medium">{redirectUrl}</span> to continue your
3655
conversation.
3756
</p>
3857
)}

apps/chat/src/components/branch-picker.tsx

Lines changed: 26 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,22 +1,36 @@
11
'use client'
22

3+
import { getBranches } from '@/src/lib/api/utils'
4+
import {
5+
type ExtendedThreadMessageLike,
6+
useChatStore,
7+
} from '@/src/stores/chatStore'
38
import { ChevronLeftIcon, ChevronRightIcon } from 'lucide-react'
49
import { useCallback, useMemo } from 'react'
5-
import { useChatStore } from '../stores/chatStore'
610
import { Tooltip, TooltipContent, TooltipTrigger } from './ui/tooltip'
711

12+
const EMPTY_MESSAGES: ExtendedThreadMessageLike[] = []
13+
814
interface BranchPickerProps {
915
messageId: string
1016
className?: string
1117
}
1218

1319
export function BranchPicker({ messageId, className }: BranchPickerProps) {
14-
const { getMessageBranches, switchToBranch } = useChatStore()
15-
16-
// get branches for current message
17-
const branches = useMemo(() => {
18-
return getMessageBranches(messageId)
19-
}, [getMessageBranches, messageId])
20+
const switchToBranch = useChatStore((state) => state.switchToBranch)
21+
22+
// get all messages from the active thread to compute branches
23+
const allMessages = useChatStore((state) => {
24+
const activeThread = state.threads.find(
25+
(t) => t.id === state.activeThreadId
26+
)
27+
return activeThread?.allMessages ?? EMPTY_MESSAGES
28+
})
29+
30+
const branches = useMemo(
31+
() => getBranches(allMessages, messageId),
32+
[allMessages, messageId]
33+
)
2034

2135
const currentIndex = useMemo(() => {
2236
const directIndex = branches.findIndex((branch) => branch.id === messageId)
@@ -25,22 +39,15 @@ export function BranchPicker({ messageId, className }: BranchPickerProps) {
2539
}
2640

2741
// if no direct match, might be an assistant message, find corresponding user message
28-
const currentThread = useChatStore
29-
.getState()
30-
.threads.find((t) => t.id === useChatStore.getState().activeThreadId)
31-
if (currentThread) {
32-
const currentMessage = currentThread.allMessages.find(
33-
(m) => m.id === messageId
42+
const currentMessage = allMessages.find((m) => m.id === messageId)
43+
if (currentMessage?.role === 'assistant' && currentMessage.parentId) {
44+
return branches.findIndex(
45+
(branch) => branch.id === currentMessage.parentId
3446
)
35-
if (currentMessage?.role === 'assistant' && currentMessage.parentId) {
36-
return branches.findIndex(
37-
(branch) => branch.id === currentMessage.parentId
38-
)
39-
}
4047
}
4148

4249
return -1
43-
}, [branches, messageId])
50+
}, [branches, messageId, allMessages])
4451

4552
const switchToBranchHandler = useCallback(
4653
(branchIndex: number) => {

apps/chat/src/components/markdown-text.tsx

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ const defaultComponents = memoizeMarkdownComponents({
141141
'text-primary font-medium underline underline-offset-4',
142142
className
143143
)}
144+
target="_blank"
145+
rel="noopener noreferrer"
144146
{...props}
145147
/>
146148
),

apps/chat/src/components/message-attachments.tsx

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@ import {
88
getAttachmentPreviewSrc,
99
} from '../lib/attachments/attachmentUi'
1010
import { useChatStore } from '../stores/chatStore'
11-
import { useComposerStore } from '../stores/composerStore'
1211
import {
1312
AttachmentPlaceholder,
1413
ThreadImageViewerModal,
@@ -47,10 +46,6 @@ export function MessageAttachments({
4746
const ensureFullImageAttachments = useChatStore(
4847
(state) => state.ensureFullImageAttachments
4948
)
50-
const setAttachmentError = useComposerStore(
51-
(state) => state.setAttachmentError
52-
)
53-
5449
const [viewerAttachmentIndex, setViewerAttachmentIndex] = useState<
5550
number | null
5651
>(null)
@@ -73,8 +68,6 @@ export function MessageAttachments({
7368
return
7469
}
7570

76-
setAttachmentError(null)
77-
7871
if (hasAllImageAttachmentsHydrated(attachments)) {
7972
return
8073
}
@@ -92,7 +85,6 @@ export function MessageAttachments({
9285
const hydratedAttachments = hydratedMessage?.imageAttachments ?? []
9386

9487
if (!hasAllImageAttachmentsHydrated(hydratedAttachments)) {
95-
setAttachmentError(HYDRATION_ERROR_MESSAGE)
9688
setViewerError(HYDRATION_ERROR_MESSAGE)
9789
}
9890
} finally {
@@ -115,7 +107,6 @@ export function MessageAttachments({
115107

116108
setViewerAttachmentIndex(index)
117109
setViewerError(null)
118-
setAttachmentError(null)
119110

120111
if (openState.shouldHydrate) {
121112
await hydrateMessageAttachments()

apps/chat/src/components/thread-image-viewer-modal.tsx

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

3+
import { Markdown } from '@klicker-uzh/markdown'
34
import { Button, Modal } from '@uzh-bf/design-system'
45

56
type MessageImageAttachment = {
@@ -47,7 +48,8 @@ export function ThreadImageViewerModal({
4748

4849
const previewSrc =
4950
attachment.imageBase64 ?? attachment.imagePreviewBase64 ?? null
50-
const title = attachment.imageDescription?.trim() || 'Image attachment'
51+
const description = attachment.imageDescription?.trim() || null
52+
const title = 'Image attachment'
5153

5254
return (
5355
<Modal
@@ -63,13 +65,23 @@ export function ThreadImageViewerModal({
6365
{previewSrc ? (
6466
<img
6567
src={previewSrc}
66-
alt={title}
68+
alt={description || title}
6769
className="max-h-[70vh] w-full rounded-lg border object-contain"
6870
/>
6971
) : (
7072
<AttachmentPlaceholder />
7173
)}
7274

75+
{description ? (
76+
<Markdown
77+
content={description}
78+
withProse
79+
className={{
80+
root: 'prose prose-sm text-foreground max-w-none',
81+
}}
82+
/>
83+
) : null}
84+
7385
{isLoading ? (
7486
<p className="text-muted-foreground text-sm">Loading full image...</p>
7587
) : null}

0 commit comments

Comments
 (0)