fix: address CodeRabbit review comments
API / router (packages/api):
- open-webui: expose serializable procedures as MCP tools via .meta({ mcp })
and gate knowledge/files/notes reads behind protectedProcedure + interact so
anonymous board viewers can't enumerate private source content
- integration create: wrap integration + creator-permission + secret writes in
a single cross-driver transaction so a partial failure can't leave an
unusable integration
- integration test: scope the permission lookup to the created integration
Integration (packages/integrations):
- open-webui: log only safe metadata (not raw completion chunks) on parse
failure, flush the final buffered SSE frame before treating the stream as
truncated, and apply a default request timeout (streaming opts out)
Widget (packages/widgets):
- collapse knowledge rows by default to avoid an N+1 getKnowledgeFiles burst
- strip markdown images from assistant output to block untrusted fetches
- reconcile the selected model when the model list changes
- clear draft attachments when switching to a saved chat
- dedupe async note/chat attaches after the fetch resolves
- discard stale persist responses via a session token so a cleared chat can't
be rebound to an old Open WebUI chat id
- release the mic stream on mid-start failures and mark the recording cancelled
on unmount so partial audio isn't transcribed
This commit is contained in:
9 files changed
+253
-71
No files matched your search
@@ -213,8 +213,10 @@ export function KnowledgeRow({
|
||||
onToggleFile: (file: FileAttachment) => void;
|
||||
emptyFilesLabel: string;
|
||||
}) {
|
||||
// Default to expanded so a base's documents are visible without an extra click.
|
||||
const [expanded, setExpanded] = useState(true);
|
||||
// Start collapsed so opening the picker doesn't fire getKnowledgeFiles for
|
||||
// every knowledge base at once (an N+1 burst on large accounts). Files load
|
||||
// only when a specific row is expanded.
|
||||
const [expanded, setExpanded] = useState(false);
|
||||
const { data: files = [], isLoading } = clientApi.widget.openWebUi.getKnowledgeFiles.useQuery(
|
||||
{ integrationId, knowledgeId },
|
||||
{ enabled: expanded && Boolean(integrationId), refetchOnWindowFocus: false, retry: false },
|
||||
@@ -577,7 +579,12 @@ export function MessageBubble({
|
||||
</Text>
|
||||
) : (
|
||||
<div className={classes.markdown}>
|
||||
<ReactMarkdown skipHtml>{content}</ReactMarkdown>
|
||||
{/* skipHtml only strips raw HTML; drop markdown images too so
|
||||
untrusted model output can't trigger arbitrary browser fetches
|
||||
on render. */}
|
||||
<ReactMarkdown skipHtml components={{ img: () => null }}>
|
||||
{content}
|
||||
</ReactMarkdown>
|
||||
</div>
|
||||
)
|
||||
) : null}
|
||||
|
||||
@@ -112,10 +112,18 @@ export function Chat({ options, integrationIds, isEditMode }: WidgetComponentPro
|
||||
captureErrorMessage: t("widget.openWebUi.captureError"),
|
||||
});
|
||||
|
||||
// Default to the first available model once the list loads.
|
||||
// Keep the selection reconciled with the available models: default to the
|
||||
// first one, and reset if the current id disappears (integration change or a
|
||||
// model removed/renamed upstream) so sends don't use a stale id.
|
||||
useEffect(() => {
|
||||
if (model !== null || models.length === 0) return;
|
||||
setModel(models[0]?.id ?? null);
|
||||
if (models.length === 0) {
|
||||
if (model !== null) setModel(null);
|
||||
return;
|
||||
}
|
||||
|
||||
if (model === null || !models.some((item) => item.id === model)) {
|
||||
setModel(models[0]?.id ?? null);
|
||||
}
|
||||
}, [model, models]);
|
||||
|
||||
// Track the floating composer's height so messages can pad below it.
|
||||
@@ -505,7 +513,13 @@ export function Chat({ options, integrationIds, isEditMode }: WidgetComponentPro
|
||||
key={chat.id}
|
||||
active={chat.id === stream.activeChatId}
|
||||
label={chat.title ?? chat.id}
|
||||
onClick={() => stream.setSelectedChatId(chat.id)}
|
||||
onClick={() => {
|
||||
// Drop the draft's grounding so a loaded conversation
|
||||
// isn't answered against the previous draft's attachments.
|
||||
attachments.reset();
|
||||
setError(null);
|
||||
stream.setSelectedChatId(chat.id);
|
||||
}}
|
||||
/>
|
||||
))
|
||||
)}
|
||||
|
||||
@@ -128,7 +128,13 @@ export function useAttachments({ integrationId, onError, errorMessages }: UseAtt
|
||||
}
|
||||
try {
|
||||
const full = await utils.widget.openWebUi.getNote.fetch({ integrationId, noteId: note.id });
|
||||
setNoteItems((previous) => [...previous, { id: full.id, title: full.title, content: full.content }]);
|
||||
// A double-click can start two fetches before state updates; dedupe on the
|
||||
// resolved id so the note isn't attached (and grounded) twice.
|
||||
setNoteItems((previous) =>
|
||||
previous.some((item) => item.id === full.id)
|
||||
? previous
|
||||
: [...previous, { id: full.id, title: full.title, content: full.content }],
|
||||
);
|
||||
} catch {
|
||||
onError(errorMessages.note);
|
||||
}
|
||||
@@ -150,7 +156,13 @@ export function useAttachments({ integrationId, onError, errorMessages }: UseAtt
|
||||
const transcript = extractChatMessages(full.chat)
|
||||
.map((message) => `${message.role}: ${message.content}`)
|
||||
.join("\n");
|
||||
setChatItems((previous) => [...previous, { id: chat.id, title: chat.title ?? chat.id, transcript }]);
|
||||
// Dedupe on the resolved id so a double-click can't attach the same chat
|
||||
// (and its transcript) twice.
|
||||
setChatItems((previous) =>
|
||||
previous.some((item) => item.id === chat.id)
|
||||
? previous
|
||||
: [...previous, { id: chat.id, title: chat.title ?? chat.id, transcript }],
|
||||
);
|
||||
} catch {
|
||||
onError(errorMessages.chat);
|
||||
}
|
||||
|
||||
@@ -45,6 +45,10 @@ export function useChatStream({
|
||||
// Read inside the serialized persist loop so a burst of turns reuses the chat
|
||||
// created by the first write instead of racing on the not-yet-committed state.
|
||||
const activeChatIdRef = useRef<string | null>(null);
|
||||
// Bumped whenever the conversation is replaced (history load / new chat) so a
|
||||
// detached persist that started against an old conversation can be discarded
|
||||
// instead of rebinding the cleared chat to a stale Open WebUI chat id.
|
||||
const sessionRef = useRef(0);
|
||||
const persistInFlightRef = useRef(false);
|
||||
const pendingPersistRef = useRef<ChatMessage[] | null>(null);
|
||||
|
||||
@@ -60,6 +64,7 @@ export function useChatStream({
|
||||
// Hydrate the conversation when a history chat finishes loading.
|
||||
useEffect(() => {
|
||||
if (!loadedChat) return;
|
||||
sessionRef.current += 1;
|
||||
const loadedMessages = extractChatMessages(loadedChat.chat);
|
||||
setMessages(loadedMessages);
|
||||
messagesRef.current = loadedMessages;
|
||||
@@ -69,7 +74,7 @@ export function useChatStream({
|
||||
onChatLoaded();
|
||||
}, [loadedChat, setModel, onChatLoaded]);
|
||||
|
||||
const runPersist = async (finalMessages: ChatMessage[]) => {
|
||||
const runPersist = async (finalMessages: ChatMessage[], session: number) => {
|
||||
if (!integrationId || !model) return;
|
||||
// Native history stores plain text; image data URLs are dropped on persist.
|
||||
const title = finalMessages.find((message) => message.role === "user")?.content.slice(0, 80) ?? newChatTitle;
|
||||
@@ -84,9 +89,13 @@ export function useChatStream({
|
||||
await updateChat.mutateAsync({ integrationId, chatId, chat: payload });
|
||||
} else {
|
||||
const chat = await createChat.mutateAsync({ integrationId, ...payload });
|
||||
// The conversation was cleared/replaced while this create was in flight;
|
||||
// don't rebind the now-stale chat id onto the current conversation.
|
||||
if (session !== sessionRef.current) return;
|
||||
activeChatIdRef.current = chat.id;
|
||||
setActiveChatId(chat.id);
|
||||
}
|
||||
if (session !== sessionRef.current) return;
|
||||
await utils.widget.openWebUi.getChats.invalidate({ integrationId });
|
||||
} catch {
|
||||
// Persistence is best-effort; the conversation remains usable in-memory.
|
||||
@@ -98,6 +107,7 @@ export function useChatStream({
|
||||
// a single trailing save of the latest messages so turns don't stack
|
||||
// concurrent POSTs (or create duplicate chats on the first save).
|
||||
const persistChat = async (finalMessages: ChatMessage[]) => {
|
||||
const session = sessionRef.current;
|
||||
if (persistInFlightRef.current) {
|
||||
pendingPersistRef.current = finalMessages;
|
||||
return;
|
||||
@@ -107,7 +117,7 @@ export function useChatStream({
|
||||
let next: ChatMessage[] | null = finalMessages;
|
||||
while (next) {
|
||||
pendingPersistRef.current = null;
|
||||
await runPersist(next);
|
||||
await runPersist(next, session);
|
||||
next = pendingPersistRef.current;
|
||||
}
|
||||
} finally {
|
||||
@@ -207,6 +217,7 @@ export function useChatStream({
|
||||
};
|
||||
|
||||
const reset = () => {
|
||||
sessionRef.current += 1;
|
||||
setMessages([]);
|
||||
messagesRef.current = [];
|
||||
setActiveChatId(null);
|
||||
|
||||
@@ -107,6 +107,12 @@ export function useVoiceRecording({
|
||||
recordingTimerRef.current = window.setInterval(() => setRecordingSeconds((value) => value + 1), 1000);
|
||||
setIsRecording(true);
|
||||
} catch {
|
||||
// getUserMedia may have succeeded before AudioContext/MediaRecorder setup
|
||||
// threw; release the mic stream and context so it isn't left running.
|
||||
releaseAudioResources();
|
||||
mediaRecorderRef.current = null;
|
||||
setAnalyser(null);
|
||||
setIsRecording(false);
|
||||
onError(micErrorMessage);
|
||||
}
|
||||
};
|
||||
@@ -121,8 +127,16 @@ export function useVoiceRecording({
|
||||
mediaRecorderRef.current?.stop();
|
||||
};
|
||||
|
||||
// Tear down the microphone stream and timer on unmount.
|
||||
useEffect(() => () => releaseAudioResources(), []);
|
||||
// Tear down the microphone stream and timer on unmount. Mark the session
|
||||
// cancelled first so a recorder "stop" fired while stopping the tracks doesn't
|
||||
// upload/transcribe partial audio after the component is gone.
|
||||
useEffect(
|
||||
() => () => {
|
||||
recordingCancelledRef.current = true;
|
||||
releaseAudioResources();
|
||||
},
|
||||
[],
|
||||
);
|
||||
|
||||
return {
|
||||
isRecording,
|
||||
|
||||
Reference in new issue
Block a user