feat: extract @hypr/helpchat shared package for Chatwoot integration#4022
feat: extract @hypr/helpchat shared package for Chatwoot integration#4022devin-ai-integration[bot] wants to merge 3 commits intomainfrom
Conversation
- Create packages/helpchat with types, client, events, and hooks - Refactor desktop useChatwootPersistence to use shared useHelpChat hook - Simplify useChatwootEvents to re-export from shared package - Wire Chatwoot persistence into support chat tab-content.tsx - Platform-agnostic design with custom fetchFn for Tauri/browser support Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
✅ Deploy Preview for hyprnote-storybook canceled.
|
❌ Deploy Preview for hyprnote failed.
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| lastPersistedCountRef.current = messages.length; | ||
|
|
||
| for (const msg of newMessages) { | ||
| const textContent = msg.parts | ||
| .filter((p): p is Extract<typeof p, { type: "text" }> => p.type === "text") | ||
| .map((p) => p.text) | ||
| .join(""); | ||
|
|
||
| if (!textContent) { | ||
| continue; | ||
| } | ||
|
|
||
| if (msg.role === "user") { | ||
| chatwoot.persistUserMessage(textContent).catch(console.error); | ||
| } else if (msg.role === "assistant") { | ||
| chatwoot.persistAgentMessage(textContent).catch(console.error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Race condition: Messages can be silently lost on persistence failure
The lastPersistedCountRef.current is updated synchronously (line 198) before the async persistUserMessage and persistAgentMessage operations complete. If any persist operation fails, those messages will never be retried because the ref already advanced past them.
// Current problematic code:
lastPersistedCountRef.current = messages.length; // Updated immediately
for (const msg of newMessages) {
if (msg.role === "user") {
chatwoot.persistUserMessage(textContent).catch(console.error); // Fails silently
}
}Fix: Only update the ref after successful persistence, or track failed messages:
const persistPromises: Promise<void>[] = [];
for (const msg of newMessages) {
const textContent = msg.parts
.filter((p): p is Extract<typeof p, { type: "text" }> => p.type === "text")
.map((p) => p.text)
.join("");
if (!textContent) continue;
if (msg.role === "user") {
persistPromises.push(chatwoot.persistUserMessage(textContent));
} else if (msg.role === "assistant") {
persistPromises.push(chatwoot.persistAgentMessage(textContent));
}
}
Promise.all(persistPromises)
.then(() => {
lastPersistedCountRef.current = messages.length;
})
.catch(console.error);| lastPersistedCountRef.current = messages.length; | |
| for (const msg of newMessages) { | |
| const textContent = msg.parts | |
| .filter((p): p is Extract<typeof p, { type: "text" }> => p.type === "text") | |
| .map((p) => p.text) | |
| .join(""); | |
| if (!textContent) { | |
| continue; | |
| } | |
| if (msg.role === "user") { | |
| chatwoot.persistUserMessage(textContent).catch(console.error); | |
| } else if (msg.role === "assistant") { | |
| chatwoot.persistAgentMessage(textContent).catch(console.error); | |
| } | |
| } | |
| const persistPromises: Promise<void>[] = []; | |
| for (const msg of newMessages) { | |
| const textContent = msg.parts | |
| .filter((p): p is Extract<typeof p, { type: "text" }> => p.type === "text") | |
| .map((p) => p.text) | |
| .join(""); | |
| if (!textContent) { | |
| continue; | |
| } | |
| if (msg.role === "user") { | |
| persistPromises.push(chatwoot.persistUserMessage(textContent)); | |
| } else if (msg.role === "assistant") { | |
| persistPromises.push(chatwoot.persistAgentMessage(textContent)); | |
| } | |
| } | |
| Promise.all(persistPromises) | |
| .then(() => { | |
| lastPersistedCountRef.current = messages.length; | |
| }) | |
| .catch(console.error); |
Spotted by Graphite Agent
Is this helpful? React 👍 or 👎 to let us know.
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| function makeClient(config: HelpChatConfig) { | ||
| const headers: Record<string, string> = {}; | ||
| if (config.accessToken) { | ||
| headers.Authorization = `Bearer ${config.accessToken}`; | ||
| } | ||
| return createClient({ baseUrl: config.apiBaseUrl, headers }); | ||
| } |
There was a problem hiding this comment.
🟡 makeClient ignores config.fetchFn, so custom fetch (e.g. tauriFetch) is only used for SSE, not API calls
The HelpChatConfig type defines a fetchFn option specifically for platform-agnostic HTTP support, and the desktop app passes tauriFetch as fetchFn at apps/desktop/src/hooks/useChatwootPersistence.ts:23. The events.ts:19 correctly uses config.fetchFn ?? globalThis.fetch for the SSE stream. However, makeClient in client.ts does not forward config.fetchFn to createClient, despite createClient supporting a fetch option (see packages/api-client/src/generated/client/types.gen.ts:25 and packages/api-client/src/generated/client/client.gen.ts:38).
Root Cause and Impact
All API functions in client.ts (createOrFindContact, fetchConversations, createNewConversation, fetchMessages, persistMessage) call makeClient(config) which constructs the client as:
return createClient({ baseUrl: config.apiBaseUrl, headers });This always falls back to globalThis.fetch for API calls, silently ignoring the fetchFn the caller explicitly provided. Meanwhile, events.ts:19 correctly does:
const fetchFn: FetchFn = config.fetchFn ?? globalThis.fetch;This inconsistency partially defeats the purpose of the fetchFn config. The PR description states the design goal is "Platform-agnostic: accepts optional fetchFn in config to support both browser fetch and Tauri's custom HTTP plugin", but this only holds for the SSE stream, not the REST API calls. Any future consumer on a platform where globalThis.fetch is unavailable or behaves differently would silently get broken API calls while SSE works correctly.
| function makeClient(config: HelpChatConfig) { | |
| const headers: Record<string, string> = {}; | |
| if (config.accessToken) { | |
| headers.Authorization = `Bearer ${config.accessToken}`; | |
| } | |
| return createClient({ baseUrl: config.apiBaseUrl, headers }); | |
| } | |
| function makeClient(config: HelpChatConfig) { | |
| const headers: Record<string, string> = {}; | |
| if (config.accessToken) { | |
| headers.Authorization = `Bearer ${config.accessToken}`; | |
| } | |
| return createClient({ baseUrl: config.apiBaseUrl, headers, fetch: config.fetchFn }); | |
| } | |
Was this helpful? React with 👍 or 👎 to provide feedback.
| const noopHandler = useCallback(() => {}, []); | ||
|
|
||
| useAgentEvents({ | ||
| config, | ||
| pubsubToken: contact?.pubsubToken ?? null, | ||
| conversationId: conversation.conversationId, | ||
| onAgentMessage: onHumanAgentMessage ?? noopHandler, | ||
| }); |
There was a problem hiding this comment.
🚩 SSE connection is always established even when no handler is wired up
In packages/helpchat/src/hooks.ts:248-254, useHelpChat always calls useAgentEvents, passing a noopHandler when onHumanAgentMessage is undefined. In the current desktop usage (tab-content.tsx:61-64), onHumanAgentMessage is never provided, so a real SSE connection is established to the server on every support chat open, but all received messages are silently discarded by the noop. The PR description acknowledges this (onHumanAgentMessage callback is plumbed through but not used), but the unnecessary network connection is worth noting — consider conditionally connecting only when a handler is provided.
Was this helpful? React with 👍 or 👎 to provide feedback.
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
Co-Authored-By: yujonglee <yujonglee.dev@gmail.com>
|
|
||
| const newMessages = messages.slice(lastPersistedCountRef.current); | ||
| if (newMessages.length === 0) { | ||
| return; | ||
| } | ||
|
|
||
| lastPersistedCountRef.current = messages.length; |
There was a problem hiding this comment.
🟡 lastPersistedCountRef skips persisting regenerated assistant messages
When a user triggers regenerate (wired to onReload in ChatBody), the last assistant message is removed from the messages array and a new streaming response begins. However, lastPersistedCountRef still holds the old higher count, so the regenerated message is never persisted to Chatwoot.
Root Cause and Walkthrough
Consider the following sequence:
messages = [user1, asst1, user2, asst2]→lastPersistedCountRef.current = 4- User clicks regenerate →
asst2is removed →messages = [user1, asst1, user2](length 3) - Status becomes
"streaming"→ the effect at line 192 returns early - Streaming completes →
messages = [user1, asst1, user2, asst2_new](length 4), status ="ready" - Effect runs:
newMessages = messages.slice(4)→[]→ returns early at line 202 lastPersistedCountRefis never updated (stays at 4), andasst2_newis never persisted to Chatwoot
The ref-based counter assumes messages only grow monotonically. Any operation that shrinks or replaces the array (like regenerate) permanently desynchronizes the counter. Since messages.slice(N) returns [] when N >= messages.length, all subsequent messages at indices ≤ the old count are silently dropped.
Impact: After any message regeneration, the Chatwoot dashboard will be missing the regenerated assistant response, giving support agents an incomplete view of the conversation.
Prompt for agents
In apps/desktop/src/components/main/body/chat/tab-content.tsx, the useEffect at lines 192-232 uses lastPersistedCountRef as a simple monotonic counter to track which messages have been persisted. This breaks when messages are regenerated (array shrinks then grows). Instead of tracking by count/index, track by message IDs. Replace lastPersistedCountRef with a Set<string> (e.g., persistedMessageIdsRef) that stores the IDs of already-persisted messages. In the effect body, filter newMessages as messages that are not in the set, and after persisting each message, add its ID to the set. This approach is resilient to array reordering, shrinking, or replacement.
Was this helpful? React with 👍 or 👎 to provide feedback.
| function makeClient(config: HelpChatConfig) { | ||
| const headers: Record<string, string> = {}; | ||
| if (config.accessToken) { | ||
| headers.Authorization = `Bearer ${config.accessToken}`; | ||
| } | ||
| return createClient({ baseUrl: config.apiBaseUrl, headers }); | ||
| } |
There was a problem hiding this comment.
🚩 fetchFn is only used for SSE streaming, not for API client calls
The HelpChatConfig.fetchFn is accepted and passed through to connectEventStream in packages/helpchat/src/events.ts:19, but makeClient at packages/helpchat/src/client.ts:18-24 does not forward config.fetchFn to createClient(). The @hypr/api-client client supports a fetch option (packages/api-client/src/generated/client/client.gen.ts:38), so it would be straightforward to pass it.
This is not a regression — the old useChatwootPersistence also used the default createClient (with globalThis.fetch) for API calls and only used tauriFetch for the SSE stream. However, the PR description states the package is "platform-agnostic" and accepts fetchFn to support both browser fetch and Tauri's HTTP plugin. A future consumer might reasonably expect fetchFn to apply to all HTTP calls, not just SSE. Consider either passing fetchFn through to makeClient for consistency, or documenting that it only affects the event stream.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
feat: extract @hypr/helpchat shared package for Chatwoot integration
Summary
Extracts Chatwoot integration logic from desktop-specific hooks into a new platform-agnostic
@hypr/helpchatpackage underpackages/helpchat/. This package can be shared betweenapps/desktopandapps/web(for the future/app/helproute). The desktop hooks become thin wrappers that inject Tauri-specific config (custom fetch, auth).New package (
packages/helpchat/):types.ts– shared type definitions (ChatwootContact, HelpChatConfig, AgentMessage, etc.)client.ts– API client functions wrapping@hypr/api-clientfor contacts, conversations, and messagesevents.ts– SSE event stream handler for real-time human agent messageshooks.ts– React hooks:useChatwootContact,useConversation,useAgentEvents,useHelpChat(composite)Desktop changes:
useChatwootPersistence.ts→ thin wrapper injecting Tauri fetch + auth config intouseHelpChatuseChatwootEvents.ts→ re-exports from shared packagetab-content.tsx→ wires persistence into support chat tab to persist user/agent messages to ChatwootDesign notes:
fetchFnin config to support both browserfetchand Tauri's custom HTTP pluginuseConversationcan automatically resume the latest conversation on mountuseHelpChatcombines contact, conversation, and event stream managementReview & Testing Checklist for Human
useChatwootPersistenceoruseChatwootEventsexist – The return type and API changed. Search codebase for other imports.tab-content.tsx:192-220– UseslastPersistedCountRefto track which messages have been persisted. Check for edge cases: messages sent during streaming, rapid status changes, or ifmessagesarray is ever replaced (not appended).initRefandresumeAttemptedRefguards in hooks prevent re-initialization ifuserIdchanges. Verify this doesn't break logout/login flows.onHumanAgentMessagecallback is passed but not used intab-content.tsx).Test Plan
Notes
onHumanAgentMessagecallback is plumbed through but not actually used intab-content.tsx– human agent messages won't appear in the UI yet^19.2.3– may need to broaden if web app uses different version.catch(console.error)) – failures are logged but not surfaced to useronHumanAgentMessageis not provided (noop handler) – could be optimizedUpdates since last revision
events.tsto pass dprint formatting checksLink to Devin run: https://app.devin.ai/sessions/02779f6a024140b49910f75592007a76
Requested by: @yujonglee