Rough Mobile/Desktop Claude Code Implementation#1041
Conversation
📝 WalkthroughWalkthroughIntroduces a real-time multiplayer AI chat system integrating Claude Code SDK, Durable Streams, and persistent session management. Adds a new streams microservice, desktop and mobile chat UIs, tRPC-based session control, schema definitions, and environment configuration across the monorepo. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Desktop as Desktop App
participant DesktopTRPC as tRPC Router
participant SessionMgr as Session Manager
participant DurableStream as Durable Stream
participant Claude as Claude SDK
User->>Desktop: Click "Ask AI..." / Start Chat
Desktop->>DesktopTRPC: startSession(sessionId, cwd)
DesktopTRPC->>SessionMgr: startSession()
SessionMgr->>DurableStream: Connect & create stream
SessionMgr->>Claude: Query with V1 streaming
Claude-->>SessionMgr: Stream events (text_delta, tool_use, etc.)
SessionMgr->>DurableStream: Persist chunks & SDK messages
SessionMgr-->>DesktopTRPC: Emit streamEvents (subscription)
DesktopTRPC-->>Desktop: Push ClaudeStreamEvent
Desktop->>Desktop: Render message with materializeMessages()
User->>Desktop: See live message stream
sequenceDiagram
participant User
participant Mobile as Mobile App
participant StreamsClient as Streams Client
participant StreamsServer as Streams Server
participant Storage as Session Storage
User->>Mobile: Open Chat List
Mobile->>StreamsClient: listSessions()
StreamsClient->>StreamsServer: GET /sessions
StreamsServer->>Storage: Load sessions.json
Storage-->>StreamsServer: SessionInfo[]
StreamsServer-->>StreamsClient: JSON response
StreamsClient-->>Mobile: SessionInfo[]
Mobile->>Mobile: Render chat list
User->>Mobile: Create new chat
Mobile->>StreamsClient: createSession({title})
StreamsClient->>StreamsServer: POST /sessions {sessionId,title}
StreamsServer->>Storage: Persist session
StreamsServer-->>StreamsClient: 201 + SessionInfo
StreamsClient-->>Mobile: SessionInfo
Mobile->>Mobile: Navigate to chat detail
sequenceDiagram
participant ChatUI as Chat UI
participant DurableChatClient as DurableChat Client
participant Collection as TanStack Collection
participant DurableStream as Durable Stream
participant Materialize as Materialize Logic
ChatUI->>DurableChatClient: connect() on mount
DurableChatClient->>DurableStream: Connect to session stream
DurableStream-->>DurableChatClient: Load existing chunks
DurableChatClient->>Collection: Subscribe to chunks collection
Collection-->>DurableChatClient: Emit updates on changes
ChatUI->>ChatUI: useCollectionData(chunks)
ChatUI->>Materialize: materializeMessages(chunks)
Materialize-->>ChatUI: MessageRow[]
ChatUI->>ChatUI: Render messages
ChatUI->>DurableChatClient: sendMessage(content)
DurableChatClient->>DurableStream: Append user_input chunk
DurableStream-->>Collection: Update chunks
Collection-->>ChatUI: Re-render via subscription
Estimated code review effort🎯 5 (Critical) | ⏱️ ~120 minutes This PR introduces a substantial feature spanning three applications with significant complexity: new streaming architecture (Claude SDK integration, Durable Streams client/server), session management across desktop/mobile, rich component hierarchies with state synchronization, authentication handling (OAuth/API keys/keychain), binary bundling and lifecycle management, and database schema implications. The changes are heterogeneous—ranging from infrastructure (streams server, Docker, Fly.io config), backend logic (session managers, tRPC routers, authentication), to frontend UI (multiple chat components, hooks, state management). Code density is high in session-manager, materialize, and client modules. The PR involves coordinated changes across 80+ files with varied patterns (new modules, refactors, config additions), making thorough review essential. Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
Note
Due to the large number of review comments, Critical, Major severity comments were prioritized as inline comments.
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 29-35: The mutations calling claudeSessionManager (the
startSession, interrupt, and stopSession mutation handlers) currently invoke
async methods without error handling; wrap each call in a try/catch, log the
error with context (include identifying data such as input.sessionId,
input.claudeSessionId, input.cwd), and surface the failure to the caller by
throwing a TRPCError (e.g., code: 'INTERNAL') or returning a failure result.
Update the handlers that call claudeSessionManager.startSession,
claudeSessionManager.interrupt, and claudeSessionManager.stopSession to follow
this pattern and use the existing logger in scope (e.g., ctx.logger or
processLogger) for the contextual log message.
In `@apps/desktop/src/main/lib/claude/auth.ts`:
- Around line 15-31: buildClaudeEnv currently ignores ClaudeCredentials with
source: "config" even when they contain a raw API key; update buildClaudeEnv to
accept config-sourced credentials and extract API keys from ClaudeConfigFile
variants. Specifically, inside buildClaudeEnv (and any helper that filters
credentials), stop excluding entries where credential.source === "config" and
instead resolve apiKey from ClaudeConfigFile fields (apiKey, api_key,
oauthAccessToken, oauth_access_token, or claudeAiOauth.accessToken) on the
credential.config object; preserve existing precedence (env > config > keychain)
and fallback logic but ensure config-provided raw keys are turned into the same
env variables used for CLI auth.
In `@apps/desktop/src/main/lib/claude/session-manager.ts`:
- Around line 449-459: The code currently only emits a "message_complete" when
session.accumulatedContent is non-empty and relies on session.currentMessageId
inside emitEvent to produce the final "done" chunk, which can leave clients
waiting for tool-only responses; remove the accumulatedContent gate so you
always emit a MessageCompleteEvent (use empty string when there's no content)
and update the emitEvent invocation/logic so it will produce the final done
chunk even if session.currentMessageId is missing for tool-only responses (e.g.,
use session.currentMessageId || generate/assign a safe ephemeral id or adjust
emitEvent to treat tool-only message_complete events as terminal and send the
done chunk).
- Around line 364-366: The console.log in processUserMessage is printing user
content (uses sessionId and content.slice) which may expose sensitive data;
replace this direct logging with either (a) a gated debug flag check (e.g., only
log when DEBUG or a sessionManager.debugMode is true) or (b) redact the message
(log length or hashed/obfuscated snippet instead of raw content), and apply the
same change to the other similar logs referenced around the 500-504 area; ensure
you update or centralize logging so sensitive fields are never written in
production logs.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/chat/`$chatId/page.tsx:
- Line 10: Replace the hardcoded STREAM_SERVER_URL constant with the environment
variable NEXT_PUBLIC_STREAMS_URL: update the declaration of STREAM_SERVER_URL
(in the page component where it's defined) to read from
process.env.NEXT_PUBLIC_STREAMS_URL (with a sensible fallback or throw if
missing) so the app uses the configured runtime URL instead of
"http://localhost:8080".
In `@apps/desktop/src/renderer/routes/_authenticated/_dashboard/chat/page.tsx`:
- Around line 19-34: Both handleCreateChat and handleSelectChat call
createStream(STREAM_SERVER_URL, ...) without handling failures; wrap the
createStream calls in try/catch blocks inside handleCreateChat and
handleSelectChat, log the error and surface user feedback (e.g., set an error
state or call a toast/notification) if createStream throws, and only call
navigate({ to: "/chat/$chatId", ... }) when the stream creation succeeds;
include references to createSession, STREAM_SERVER_URL, navigate so you update
the correct handlers.
- Line 9: Replace the hardcoded STREAM_SERVER_URL constant with the
environment-backed value from env.renderer.ts: import and use the
NEXT_PUBLIC_STREAMS_URL variable (as used in chatStore.ts) instead of
"http://localhost:8080"; update the STREAM_SERVER_URL declaration in the
affected files (the STREAM_SERVER_URL constant in page.tsx and ChatView.tsx) to
reference NEXT_PUBLIC_STREAMS_URL so the runtime/build-injected value is used in
production.
In `@apps/desktop/src/renderer/screens/chat/ChatView.tsx`:
- Around line 46-52: The handler handleStartSession currently passes a hardcoded
cwd ("/Users/satyapatel/code/superset") into startSessionMutation; remove that
literal and instead accept or derive the working directory: update ChatViewProps
to include a required cwd prop (or obtain it from the workspace/context
provider) and use that prop when calling startSessionMutation.mutateAsync({
sessionId, cwd }); ensure you also update any callers of ChatView to provide cwd
and keep sessionId, startSessionMutation and refetchIsActive usage unchanged.
- Around line 70-82: allMessages currently narrows m.role with `as "user" |
"assistant"` which silently drops the "system" MessageRole; update the mapping
inside the useMemo (references: allMessages, messages, streamingMessage) to
explicitly filter out or handle MessageRole === "system" before casting — e.g.,
skip system messages or map them to an allowed role/structure so only safe roles
reach ChatMessageList — and ensure the resulting array type remains
Array<Message | StreamingMessage>.
In
`@apps/desktop/src/renderer/screens/chat/components/ChatMessageList/ChatMessageList.tsx`:
- Around line 37-41: The useEffect that scrolls to bottom only depends on
autoScroll, so it won't run when new messages arrive; update the dependency
array for the effect in ChatMessageList (the useEffect that references
autoScroll and bottomRef) to include the messages array (or a suitable messages
length/identifier) so the effect re-runs when messages change and triggers
bottomRef.current.scrollIntoView({ behavior: "smooth" }) when autoScroll is
true.
In `@apps/streams/data/sessions.json`:
- Around line 1-16: This file contains mutable runtime session records (keys
"sessionId", "title", "createdAt") that should not be committed; remove the
real/dev-generated entries from the repository and either (a) move a synthetic
fixture/seed file into a dedicated fixtures/ or seed/ folder with clearly fake
UUIDs and timestamps and update any test/seed loader to reference that file (use
keys "sessionId"/"title"/"createdAt" for compatibility), or (b) delete the
sessions JSON from the repo and add the filename pattern to .gitignore and
remove it from git history/index (git rm --cached) so runtime data is not
tracked; ensure any consumer code that referenced this file points to the new
fixture or dynamic session store.
In `@apps/streams/Dockerfile`:
- Around line 6-8: The Dockerfile currently copies bun.lock but runs npm
install; update both builder and production stages to use Bun's frozen lockfile
install: replace usages of "npm install" (in the builder stage run block) with
"bun install --frozen-lockfile" and ensure bun.lock is copied into the
production stage as well (add bun.lock to the COPY that seeds the runtime image)
so builds are deterministic; locate the COPY and RUN lines in the Dockerfile
that reference package.json/bun.lock and npm install and change them
accordingly.
In `@apps/streams/src/index.ts`:
- Around line 168-175: durableServer.start() is called without handling
rejections, so failures are unhandled; wrap the startup in a promise chain or
async/await that handles errors from durableServer.start() (reference
durableServer.start) and log the error (use console.error or the project logger)
and exit with non-zero status if startup fails; ensure server.listen(...)
(reference server.listen and port) only runs after a successful
durableServer.start() and add a .catch(...) (or try/catch) to handle and report
the error and call process.exit(1).
In
`@apps/web/src/app/`(dashboard)/chat/[sessionId]/components/ChatRoom/ChatRoom.tsx:
- Around line 33-38: Destructure streamingMessage from the useChatSession call
(alongside users, draft, setDraft) and integrate it into the computed
allMessages array used for rendering (e.g., include streamingMessage at the end
or merged into messages when present) so live streaming content appears
immediately; update references to allMessages consumers accordingly and ensure
the draft handling (draft, setDraft) remains unchanged.
In
`@apps/web/src/app/`(dashboard)/chat/components/ChatMessageList/ChatMessageList.tsx:
- Around line 43-46: The auto-scroll useEffect in the ChatMessageList component
only runs on mount because its dependency array is empty; update the effect that
calls bottomRef.current?.scrollIntoView(...) to depend on message changes (e.g.,
add messages or messages.length to the dependency array) so it re-runs when new
messages arrive; locate the useEffect in ChatMessageList.tsx and replace the
empty deps with an appropriate messages-based dependency to ensure scrolling on
new messages.
In
`@apps/web/src/app/`(dashboard)/chat/components/ChatSessionList/ChatSessionList.tsx:
- Around line 26-38: The function formatRelativeTime uses magic number literals
for milliseconds (60000, 3600000, 86400000); extract these into descriptive
module-level constants (e.g., MS_PER_MINUTE, MS_PER_HOUR, MS_PER_DAY) at the top
of the file and replace the literals in formatRelativeTime with those constants
so the conversions read MS_PER_MINUTE, MS_PER_HOUR, and MS_PER_DAY respectively,
keeping the logic unchanged.
In `@packages/ai-chat/src/stream/actions.ts`:
- Around line 17-28: Update createStream to accept a single params object (e.g.,
createStream({ baseUrl, sessionId }): Promise<boolean>) and add proper error
handling: perform the PUT as before, return true only for 201, return false for
the known "already exists" success code (200 or explicit 409 if your API uses
that), and for any other non-2xx response throw an Error that includes the
response.status and response text/body; also catch and rethrow network/fetch
errors with a clear message so callers can distinguish server errors from
"already exists". Use the function name createStream and parameter names baseUrl
and sessionId to locate the code to change.
In `@packages/ai-chat/src/stream/schema.ts`:
- Around line 64-69: The chunks collection in sessionStateSchema declares
primaryKey: "id" but chunkSchema has no id field (only messageId, actorId, role,
chunk, seq, createdAt), so update the schema: either add a unique id field to
chunkSchema (e.g., id) and ensure it’s populated when creating chunks, or change
sessionStateSchema.chunks.primaryKey to use an existing unique field (e.g.,
"messageId" or a supported composite key like ["messageId","seq"]) so the
primary key matches an actual field defined on chunkSchema; adjust any
creation/lookup code that relies on the primary key (functions that write/read
chunks) to use the chosen key.
In `@packages/trpc/src/router/chat/chat.ts`:
- Around line 498-515: The current code only checks that the session exists in
the organization but does not verify the caller's role; update the handler after
the chatSessions query to ensure the authenticated user (current user id) is a
participant on the session with role "owner" or "editor" before allowing adding
participants. Query the session-participants table (e.g.,
chatSessionParticipants) for a row matching input.sessionId and the current
user's id and assert participant.role is "owner" or "editor"; if not, throw a
TRPCError with code "FORBIDDEN" and a descriptive message. Ensure you reference
the existing variables session, input.sessionId and organizationId and use the
same db instance for the additional query.
🟡 Minor comments (15)
apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx-134-150 (1)
134-150: Chat button should be gated behind a feature flag consistent with Tasks.Tasks is conditionally rendered based on the
ELECTRIC_TASKS_ACCESSfeature flag, but the Chat button is always rendered. No correspondinghasChatAccessfeature flag is defined inFEATURE_FLAGS. Either add a feature flag for Chat to enable controlled rollout (consistent with Tasks), or document why Chat intentionally lacks this gate.packages/ai-chat/src/components/PresenceBar/PresenceBar.tsx-15-22 (1)
15-22: Handle edge case for empty name parts in getInitials.If the name contains consecutive spaces (e.g.,
"John Doe"),split(" ")produces empty strings, andn[0]would beundefined, causing the result to include "undefined".🐛 Proposed fix
function getInitials(name: string): string { return name .split(" ") + .filter(Boolean) .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2); }packages/ai-chat/src/components/ChatInput/ChatInput.tsx-76-103 (1)
76-103: Missing cleanup for typing timeout on unmount.The
typingTimeoutReftimeout is not cleared when the component unmounts, which could causeonTypingChangeto be called on an unmounted component.🐛 Suggested fix: add cleanup effect
Add a cleanup effect after the
typingTimeoutRefdeclaration:import { type KeyboardEvent, useCallback, useEffect, useRef, useState } from "react"; // ... after line 40 useEffect(() => { return () => { if (typingTimeoutRef.current) { clearTimeout(typingTimeoutRef.current); } }; }, []);packages/ai-chat/src/stream/client.ts-202-212 (1)
202-212: Silently swallowed error violates coding guidelines.Line 205 uses
.catch(() => {})which silently discards any error from_removePresence(). At minimum, log the error with context.🔧 Proposed fix
disconnect(): void { // Remove presence before disconnecting if (this.options.user && this._isConnected) { - this._removePresence().catch(() => {}); + this._removePresence().catch((error) => { + console.error("[ai-chat/client] Failed to remove presence on disconnect:", error); + }); } this._db.close();As per coding guidelines: "Never silently swallow errors with
catch(() => {})orcatch(e) { return null }" and "Log errors at minimum if not re-throwing or explicitly handling them".apps/desktop/src/renderer/routes/_authenticated/_dashboard/chat/$chatId/page.tsx-28-40 (1)
28-40: Stream creation failures are not handled.Both
handleCreateChatandhandleSelectChatproceed with navigation even ifcreateStreamfails. This could leave the user on a chat page without a functioning stream.🔧 Proposed fix with error handling
const handleCreateChat = useCallback(async () => { const session = createSession(); - await createStream(STREAM_SERVER_URL, session.id); - navigate({ to: "/chat/$chatId", params: { chatId: session.id } }); + try { + await createStream(STREAM_SERVER_URL, session.id); + navigate({ to: "/chat/$chatId", params: { chatId: session.id } }); + } catch (error) { + console.error("[chat/createChat] Failed to create stream:", error); + // Optionally show user feedback + } }, [navigate, createSession]); const handleSelectChat = useCallback( async (id: string) => { - await createStream(STREAM_SERVER_URL, id); - navigate({ to: "/chat/$chatId", params: { chatId: id } }); + try { + await createStream(STREAM_SERVER_URL, id); + navigate({ to: "/chat/$chatId", params: { chatId: id } }); + } catch (error) { + console.error("[chat/selectChat] Failed to create stream:", error); + } }, [navigate], );As per coding guidelines: "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern
[domain/operation] message".apps/desktop/package.json-37-37 (1)
37-37: Consider updating to the latest Claude Agent SDK version.The specified version
^0.2.19is not the latest; version0.2.22is currently available on npm. While the caret prefix allows automatic minor/patch updates, consider updating to0.2.22to ensure you're using the most recent stable release, especially since this package is in active development (0.x range).apps/desktop/package.json-56-58 (1)
56-58: Consider updating to the latest TanStack package versions.The specified versions are available (
@tanstack/db@0.5.23and@tanstack/react-db@0.1.67), but newer patch releases exist:@tanstack/db@0.5.24and@tanstack/react-db@0.1.68. Since TanStack DB is in BETA, consider pinning to the latest stable patch versions to avoid missing bug fixes or security updates.apps/desktop/src/main/lib/claude/binary.ts-48-61 (1)
48-61: Docstring contradicts implementation.The docstring states "Returns null if no binary is available" but the function returns the string
"claude"as a fallback, nevernull. Either update the docstring or change the implementation to match.📝 Option 1: Fix docstring to match behavior
/** * Get the Claude binary path, falling back to PATH if bundled binary doesn't exist. - * Returns null if no binary is available. + * Falls back to "claude" for PATH resolution if bundled binary doesn't exist. */ export function getClaudeBinaryPath(): string | null {📝 Option 2: Change implementation to match docstring
export function getClaudeBinaryPath(): string | null { // First try bundled binary if (hasBundledClaudeBinary()) { return getBundledClaudeBinaryPath(); } - // Fall back to checking if claude is in PATH (for development) - // We don't resolve it here - just return "claude" and let the shell find it - return "claude"; + // No bundled binary available + return null; }apps/mobile/screens/(authenticated)/chat/ChatScreen.tsx-108-128 (1)
108-128: Remove unnecessarystreamsUrlfrom dependency array.Static analysis correctly identifies that
streamsUrlis derived fromenv.EXPO_PUBLIC_STREAMS_URL, which is a module-level constant that won't change during the component's lifecycle. Including it causes unnecessary re-renders of the callback.Suggested fix
const handleSend = useCallback( async (content: string) => { if (!user?.userId || !streamsUrl || !sessionId) { console.error( "[chat] Cannot send message: missing user, URL, or sessionId", ); return; } try { await sendMessageToStream({ proxyUrl: streamsUrl, sessionId, userId: user.userId, content, }); } catch (err) { console.error("[chat] Failed to send message:", err); } }, - [user?.userId, streamsUrl, sessionId], + [user?.userId, sessionId], );apps/mobile/lib/streams/client.ts-91-119 (1)
91-119: Race condition ingetOrCreateSessionbetween check and create.The get-then-create pattern is susceptible to TOCTOU (time-of-check to time-of-use) race conditions. If two clients call
getOrCreateSessionwith the samesessionIdsimultaneously, both may seeexistingasnulland attempt to create, potentially causing a conflict or duplicate creation on the server.Consider either:
- Using an atomic upsert endpoint on the server
- Handling 409 Conflict responses gracefully by re-fetching
Suggested improvement for handling race condition
export async function getOrCreateSession({ sessionId, title, createdBy, }: { sessionId: string; title: string; createdBy?: string; }): Promise<SessionInfo> { const existing = await getSession(sessionId); if (existing) { return existing; } const url = getStreamsUrl(); const res = await fetch(`${url}/sessions`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ sessionId, title, createdBy }), }); + // Handle race condition: another client may have created the session + if (res.status === 409) { + const retryExisting = await getSession(sessionId); + if (retryExisting) { + return retryExisting; + } + throw new Error("Session conflict but session not found on retry"); + } + if (!res.ok) { throw new Error( `Failed to create session: ${res.status} ${res.statusText}`, ); } return res.json(); }packages/ai-chat/src/stream/materialize.ts-56-61 (1)
56-61: Don’t silently drop invalid chunk JSON.A parse failure here is worth logging with context.
💡 Suggested fix
export function parseChunkContent(chunkJson: string): ChunkContent | null { try { return JSON.parse(chunkJson) as ChunkContent; - } catch { + } catch (error) { + console.warn( + "[ai-chat/materialize] Failed to parse chunk JSON:", + error, + ); return null; } }As per coding guidelines: Never swallow errors silently; at minimum log them with context.
apps/desktop/src/main/lib/claude/session-manager.ts-231-268 (1)
231-268: Stream watcher swallows errors.Please log poll/parse failures with context so stream issues are diagnosable.
💡 Suggested fix
try { const parsed = JSON.parse(chunk); if (parsed.type === "whole-message" && parsed.content) { console.log(`[stream-watcher] New user message: ${messageId}`); this.onNewUserMessage(messageId, parsed.content); } - } catch { - // Not JSON, ignore + } catch (error) { + console.warn( + `[stream-watcher/poll] Failed to parse chunk for ${messageId}:`, + error, + ); } } - } catch { - // Ignore poll errors + } catch (error) { + console.warn( + `[stream-watcher/poll] Poll failed for ${this.sessionId}:`, + error, + ); } }As per coding guidelines: Never swallow errors silently; at minimum log them with context.
apps/desktop/src/main/lib/claude/auth.ts-228-235 (1)
228-235: OAuth credential parse errors are swallowed.Log parsing failures so OAuth issues are diagnosable.
💡 Suggested fix
if (existsSync(credentialsPath)) { try { const content = readFileSync(credentialsPath, "utf-8"); const config: ClaudeConfigFile = JSON.parse(content); return !!config.claudeAiOauth?.accessToken; - } catch { + } catch (error) { + console.debug( + "[claude/auth] Failed to parse OAuth credentials file:", + error, + ); return false; } }As per coding guidelines: Never swallow errors silently; at minimum log them with context.
apps/desktop/src/main/lib/claude/auth.ts-102-132 (1)
102-132: Keychain lookup errors are swallowed.Even if “not found” is normal, unexpected failures should be logged with context.
💡 Suggested fix
try { // Claude CLI stores credentials in the keychain with this service/account const result = execSync( 'security find-generic-password -s "claude-cli" -a "api-key" -w 2>/dev/null', { encoding: "utf-8" }, ).trim(); @@ - } catch { - // Not found in keychain, this is fine + } catch (error) { + console.debug("[claude/auth] Keychain lookup failed:", error); } @@ - } catch { - // Not found in keychain, this is fine + } catch (error) { + console.debug("[claude/auth] Keychain lookup failed:", error); }As per coding guidelines: Never swallow errors silently; at minimum log them with context.
apps/desktop/src/renderer/stores/chatStore.ts-29-42 (1)
29-42: Handle non‑OK responses from the streams sync call.
fetchonly rejects on network errors; 4xx/5xx are currently silent.💡 Suggested fix
fetch(`${streamsUrl}/sessions`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify({ sessionId: session.id, title: session.name, }), -}).catch((err) => { - console.warn("[chatStore] Failed to sync session to streams server:", err); -}); +}) + .then((response) => { + if (!response.ok) { + console.warn( + `[chatStore/sync] Streams sync failed (${response.status})`, + ); + } + }) + .catch((err) => { + console.warn( + "[chatStore/sync] Failed to sync session to streams server:", + err, + ); + });As per coding guidelines: Never swallow errors silently; at minimum log them with context; Use prefixed console logging with pattern
[domain/operation] messagefor all logging.
🧹 Nitpick comments (41)
apps/desktop/src/renderer/index.html (1)
14-18: Consider restricting localhost to port 8080 if the streams server port is fixed.The wildcard
http://localhost:*allows connections to any local port, but the application consistently uses port 8080 for the durable streams server (hardcoded in chat views and defaulting viaNEXT_PUBLIC_STREAMS_URLenvironment variable). Restricting the CSP tohttp://localhost:8080would follow the principle of least privilege.While this is a low-priority concern for a desktop Electron app (where localhost is inherently trusted), making the CSP more explicit improves clarity and reduces attack surface if the port assignment changes unexpectedly.
apps/desktop/src/renderer/providers/AuthProvider/AuthProvider.tsx (1)
44-57: Improved re-authentication flow looks good, minor defensive suggestion.The async handler correctly clears old session state before setting the new token, preventing stale session data. Using
{ throw: false }is a good practice to avoid blocking the token update flow.Consider wrapping the
signOutcall in a try/catch for extra safety, in case the underlying implementation throws despite the option (e.g., network timeout errors that bypass the flag):🛡️ Optional: Add defensive try/catch
onData: async (data) => { if (data?.token && data?.expiresAt) { // New authentication - clear old session state first, then set new token setAuthToken(null); - await authClient.signOut({ fetchOptions: { throw: false } }); + try { + await authClient.signOut({ fetchOptions: { throw: false } }); + } catch { + // Proceed with token update even if sign-out fails + } setAuthToken(data.token);packages/ai-chat/src/components/PresenceBar/PresenceBar.tsx (1)
44-60: Extract magic number for max displayed viewers.The limit of 5 viewers appears twice. Extract to a named constant for clarity and single source of truth.
♻️ Proposed refactor
+const MAX_DISPLAYED_VIEWERS = 5; + export function PresenceBar({ viewers, typingUsers, className, }: PresenceBarProps) {Then update usages:
- {viewers.slice(0, 5).map((user) => ( + {viewers.slice(0, MAX_DISPLAYED_VIEWERS).map((user) => (- {viewers.length > 5 && ( + {viewers.length > MAX_DISPLAYED_VIEWERS && ( <span className="text-xs text-muted-foreground"> - +{viewers.length - 5} + +{viewers.length - MAX_DISPLAYED_VIEWERS}apps/api/src/app/api/electric/[...path]/utils.ts (1)
67-91: Consider scalability for organizations with many chat sessions.The current approach fetches all session IDs for an organization, then uses
inArrayto filter participants. For organizations with a very large number of sessions, this could:
- Return a large result set from the initial query
- Hit database parameter limits with the
inArrayclauseFor now this is likely acceptable, but consider using a subquery or join if session counts grow significantly.
💡 Alternative using subquery pattern
case "chat_participants": { - // Filter participants by sessions the user's org owns - const orgSessions = await db.query.chatSessions.findMany({ - where: eq(chatSessions.organizationId, organizationId), - columns: { id: true }, - }); - - if (orgSessions.length === 0) { - return { fragment: "1 = 0", params: [] }; - } - - const sessionIds = orgSessions.map((s) => s.id); - const whereExpr = inArray( - sql`${sql.identifier(chatParticipants.sessionId.name)}`, - sessionIds, - ); - const qb = new QueryBuilder(); - const { sql: query, params } = qb - .select() - .from(chatParticipants) - .where(whereExpr) - .toSQL(); - const fragment = query.replace(/^select .* from .* where\s+/i, ""); - return { fragment, params }; + // Filter participants by sessions belonging to the organization + const fragment = `"session_id" IN (SELECT id FROM chat_sessions WHERE organization_id = $1)`; + return { fragment, params: [organizationId] }; }packages/ai-chat/package.json (1)
23-23: Consider aligningreactdependency version with peer dependency range.The
dependenciessection pinsreactto exact version19.1.0, butpeerDependenciesdeclares compatibility with^18.0.0 || ^19.0.0. Since this is a shared library package, listingreactin dependencies (rather than only in peerDependencies) may cause version conflicts or duplicate React instances when consumers use a different React version.💡 Suggested fix: move react to peerDependencies only
"dependencies": { "@durable-streams/client": "^0.2.0", "@durable-streams/state": "^0.2.0", "@superset/ui": "workspace:*", "@tanstack/db": "^0.5.23", "@tanstack/react-db": "^0.1.67", "lucide-react": "^0.560.0", - "react": "19.1.0", "zod": "^4.3.5" },packages/ai-chat/src/components/ChatInput/ChatInput.tsx (1)
85-85: Extract magic numbers to named constants.The values
200(max textarea height) and2000(typing debounce ms) on lines 85 and 97 are magic numbers. Per coding guidelines, extract them to named constants at module top.♻️ Suggested refactor
+const MAX_TEXTAREA_HEIGHT_PX = 200; +const TYPING_DEBOUNCE_MS = 2000; + export interface ChatInputProps {Then update usages:
- textarea.style.height = `${Math.min(textarea.scrollHeight, 200)}px`; + textarea.style.height = `${Math.min(textarea.scrollHeight, MAX_TEXTAREA_HEIGHT_PX)}px`;- typingTimeoutRef.current = setTimeout(() => { - onTypingChange?.(false); - }, 2000); + typingTimeoutRef.current = setTimeout(() => { + onTypingChange?.(false); + }, TYPING_DEBOUNCE_MS);Based on coding guidelines: "Avoid magic numbers by extracting them to named constants at module top".
packages/ai-chat/src/stream/actions.ts (1)
40-42: Add logging context to error before throwing.Per coding guidelines, errors should be logged with context. Consider adding a prefixed log before throwing.
Suggested improvement
if (!response.ok) { + console.error(`[ai-chat/stream] Failed to append to stream: ${response.status}`); throw new Error(`Failed to append to stream: ${response.status}`); }apps/desktop/src/renderer/screens/chat/components/ChatMessage/ChatMessage.tsx (1)
11-16: The "streaming" role value is never used.The
roleprop accepts"streaming"but the component only checks for"user". The streaming state is handled separately viaisStreamingprop. Consider removing the unused "streaming" value to avoid confusion.Proposed simplification
export interface ChatMessageProps { - role: "user" | "assistant" | "streaming"; + role: "user" | "assistant"; content: string; timestamp?: Date; isStreaming?: boolean; }Note: The web version at
apps/web/src/app/(dashboard)/chat/components/ChatMessage/ChatMessage.tsxuses only"user" | "assistant"for role, so this would also improve cross-platform consistency.packages/ai-chat/src/stream/client.ts (2)
124-127: Suspicious double type assertion.The
as unknown as SessionCollectionscast bypasses type safety. IfcreateStreamDBreturns collections with a different shape, this will cause runtime errors without compile-time warnings.Consider adding a runtime validation or documenting why this cast is safe:
- // Collections are available immediately - this._collections = this._db.collections as unknown as SessionCollections; + // Collections are available immediately + // Note: stream-db collections match SessionCollections shape but use different generic params + this._collections = this._db.collections as unknown as SessionCollections;Or validate at runtime:
const collections = this._db.collections; if (!collections.chunks || !collections.presence || !collections.drafts) { throw new Error("StreamDB missing expected collections"); } this._collections = collections as unknown as SessionCollections;
358-371: Add prefixed logging and include response body in error.The error message only includes the status code. Including the response body would help with debugging. Also, consider adding logging per coding guidelines.
🔧 Proposed improvement
private async _appendToStream(events: unknown[]): Promise<void> { const response = await fetch( `${this.options.proxyUrl}/streams/${this.sessionId}`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(events), }, ); if (!response.ok) { - throw new Error(`Failed to append to stream: ${response.status}`); + const body = await response.text().catch(() => ""); + const message = `Failed to append to stream: ${response.status}${body ? ` - ${body}` : ""}`; + console.error(`[ai-chat/client] ${message}`); + throw new Error(message); } }As per coding guidelines: "Use prefixed console logging with pattern
[domain/operation] messagefor all logging".docs/ai-chat-plan.md (2)
7-35: Add language specifier to fenced code block.The architecture diagram code block should specify a language (e.g.,
textorplaintext) to satisfy linting rules and improve rendering consistency.-``` +```text Any Client (Web/Mobile) API Desktop (Electron)
281-290: Add language specifier to directory tree block.-``` +```text apps/stream-server/ ├── package.jsonapps/streams/fly.toml (1)
27-30: Consider if 256MB memory is sufficient.256MB is minimal for a Node.js application. While acceptable for a lightweight streaming proxy, monitor memory usage under load. If the server handles many concurrent connections or buffers messages, you may need to increase this.
apps/desktop/src/lib/trpc/routers/auth/index.ts (1)
71-75: Extract magic number to a named constant.The
10 * 60 * 1000timeout value should be extracted to a named constant at module scope for clarity.Suggested refactor
+const STATE_EXPIRY_MS = 10 * 60 * 1000; // 10 minutes + export const createAuthRouter = () => { // ... - // Clean up expired states (10 minutes) - const cutoff = Date.now() - 10 * 60 * 1000; + // Clean up expired states + const cutoff = Date.now() - STATE_EXPIRY_MS;As per coding guidelines: "Avoid magic numbers by extracting them to named constants at module top".
apps/web/src/app/(dashboard)/chat/page.tsx (2)
17-27: Consider adding error handling and loading state for session creation.The mutation lacks error handling, which means failures will be silent. Consider adding
onErroror usingcreateSessionMutation.errorto display feedback to users. Additionally, exposing the loading state would improve UX by disabling the button during creation.Suggested improvement
const createSessionMutation = useMutation( trpc.chat.createSession.mutationOptions({ onSuccess: (data) => { router.push(`/chat/${data.session.id}`); }, + onError: (error) => { + console.error("[chat/createSession] Failed to create session:", error.message); + // Consider adding toast notification here + }, }), ); const handleNewChat = useCallback(() => { + if (createSessionMutation.isPending) return; createSessionMutation.mutate({}); }, [createSessionMutation]);Then pass the loading state to the component:
<ChatSessionList sessions={sessions} onNewChat={handleNewChat} isCreating={createSessionMutation.isPending} />As per coding guidelines: "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern
[domain/operation] message".
29-34: Consider memoizing the session mapping.The session transformation runs on every render. While acceptable for small lists, memoizing with
useMemowould prevent unnecessary recalculations if the component re-renders for other reasons.Suggested refactor
+import { useCallback, useMemo } from "react"; -const sessions = sessionsQuery.data.map((row) => ({ - id: row.session.id, - title: row.session.title, - updatedAt: new Date(row.session.updatedAt), - creatorName: row.creator?.name ?? null, -})); +const sessions = useMemo( + () => + sessionsQuery.data.map((row) => ({ + id: row.session.id, + title: row.session.title, + updatedAt: new Date(row.session.updatedAt), + creatorName: row.creator?.name ?? null, + })), + [sessionsQuery.data], +);apps/desktop/src/renderer/env.renderer.ts (1)
18-20: Production default concern for NEXT_PUBLIC_STREAMS_URL.The existing API and web URLs default to production endpoints (
https://api.superset.sh,https://app.superset.sh), butNEXT_PUBLIC_STREAMS_URLdefaults tohttp://localhost:8080. This inconsistency means production builds could silently fall back to localhost if the env var isn't set.Consider either:
- Defaulting to a production streams URL for consistency, or
- Making this field required (no default) to force explicit configuration.
apps/mobile/screens/(authenticated)/chat/components/ChatMessage/ChatMessage.tsx (1)
20-24: Clarify the role label condition.The condition
!isCurrentUser && message.role !== "user"shows the role label for non-current-user messages that also aren't "user" role. This seems to target assistant/system messages, but the logic could be clearer.If intent is to show role labels only for assistant messages, consider:
{message.role === "assistant" && ( <Text className={`text-xs ${textColor} opacity-70 mb-1`}> {message.role} </Text> )}apps/desktop/src/renderer/routes/_authenticated/_dashboard/chat/$chatId/page.tsx (1)
24-26: Add logging prefix for consistency.Per coding guidelines, use prefixed console logging with pattern
[domain/operation] message.🔧 Suggested change
useEffect(() => { - createStream(STREAM_SERVER_URL, chatId).catch(console.error); + createStream(STREAM_SERVER_URL, chatId).catch((error) => { + console.error("[chat/ensureStream] Failed to ensure stream exists:", error); + }); }, [chatId]);apps/mobile/screens/(authenticated)/chat/components/ChatInput/ChatInput.tsx (1)
44-44: Extract magic color values to named constants.The hardcoded color values
"#9ca3af"and"#ffffff"appear multiple times. Consider extracting them to module-level constants for maintainability and consistency.♻️ Suggested refactor
import { Send } from "lucide-react-native"; import { useState } from "react"; import { Pressable, TextInput, View } from "react-native"; +const COLORS = { + placeholder: "#9ca3af", + sendIconActive: "#ffffff", + sendIconDisabled: "#9ca3af", +} as const; + export interface ChatInputProps {Then use
COLORS.placeholder,COLORS.sendIconActive, andCOLORS.sendIconDisabledin the component.apps/desktop/scripts/download-claude-binary.ts (3)
74-76: Refactor to avoid assignment in expression.The static analysis tool flagged
data += chunkinside an arrow expression. While this pattern works, it can be confusing as expressions are typically side-effect free.♻️ Suggested fix
- res.on("data", (chunk) => (data += chunk)); + res.on("data", (chunk) => { + data += chunk; + });Apply the same change to line 104.
59-112: Consider consolidatingfetchJsonandfetchTextto reduce duplication.These two functions share nearly identical redirect handling and error logic. A small refactor could eliminate the duplication.
♻️ Suggested approach
function fetchRaw(url: string): Promise<string> { return new Promise((resolve, reject) => { const request = (requestUrl: string) => { https .get(requestUrl, (res) => { if (res.statusCode === 301 || res.statusCode === 302) { const location = res.headers.location; if (location) { return request(location); } return reject(new Error("Redirect without location")); } if (res.statusCode !== 200) { return reject(new Error(`HTTP ${res.statusCode}`)); } let data = ""; res.on("data", (chunk) => { data += chunk; }); res.on("end", () => resolve(data)); res.on("error", reject); }) .on("error", reject); }; request(url); }); } function fetchJson<T>(url: string): Promise<T> { return fetchRaw(url).then((data) => JSON.parse(data) as T); } function fetchText(url: string): Promise<string> { return fetchRaw(url); }
217-218: Extract hardcoded fallback version to a named constant.The fallback version
"2.1.17"is a magic value. Consider extracting it and adding a comment explaining why this specific version was chosen as the fallback.♻️ Suggested fix
+// Fallback version used when latest version cannot be fetched +const FALLBACK_CLAUDE_VERSION = "2.1.17"; + // Claude Code distribution base URL (same as 1code uses) const DIST_BASE =// Fallback to known version - return "2.1.17"; + return FALLBACK_CLAUDE_VERSION;apps/desktop/src/renderer/screens/chat/ChatView.tsx (2)
20-20: Consider extracting the stream server URL to configuration.The hardcoded localhost URL will need to be configurable for different environments (development, staging, production).
💡 Suggested approach
+import { env } from "renderer/lib/env"; // or appropriate env module + -const STREAM_SERVER_URL = "http://localhost:8080"; +const STREAM_SERVER_URL = env.STREAM_SERVER_URL ?? "http://localhost:8080";
57-68: Add error logging in handleSend.Per coding guidelines, errors should not be silently swallowed. While the error is re-thrown, it should be logged with context for debugging.
💡 Proposed fix
const handleSend = useCallback( async (content: string) => { setIsSending(true); setDraft(""); try { await sendMessage(content); + } catch (err) { + console.error("[chat/send] Failed to send message:", err); + throw err; } finally { setIsSending(false); } }, [sendMessage, setDraft], );As per coding guidelines: "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern
[domain/operation] message".apps/mobile/screens/(authenticated)/chat/ChatListScreen.tsx (2)
29-38: Consider a more specific query key to avoid cache collisions.The query key
["sessions"]is generic and could potentially conflict with other session-related queries in the app (e.g., auth sessions). Consider namespacing it.💡 Proposed fix
const { data: sessions = [], isLoading, refetch, isRefetching, } = useQuery({ - queryKey: ["sessions"], + queryKey: ["chat", "sessions"], queryFn: listSessions, enabled: !!streamsUrl, });Also update the invalidation call:
-queryClient.invalidateQueries({ queryKey: ["sessions"] }); +queryClient.invalidateQueries({ queryKey: ["chat", "sessions"] });
202-218: Extract magic numbers to named constants.The time calculation thresholds use magic numbers. Per coding guidelines, these should be extracted to named constants for clarity.
💡 Proposed fix
+const MS_PER_HOUR = 60 * 60 * 1000; +const MS_PER_DAY = 24 * MS_PER_HOUR; +const MS_PER_WEEK = 7 * MS_PER_DAY; function formatDate(dateString: string): string { const date = new Date(dateString); const now = new Date(); const diff = now.getTime() - date.getTime(); // Less than 24 hours - if (diff < 24 * 60 * 60 * 1000) { + if (diff < MS_PER_DAY) { return date.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" }); } // Less than 7 days - if (diff < 7 * 24 * 60 * 60 * 1000) { + if (diff < MS_PER_WEEK) { return date.toLocaleDateString([], { weekday: "short" }); } return date.toLocaleDateString([], { month: "short", day: "numeric" }); }As per coding guidelines: "Avoid magic numbers by extracting them to named constants at module top".
apps/web/src/app/(dashboard)/chat/components/ChatMessage/ChatMessage.tsx (1)
20-27: getInitials may produce unexpected results for edge cases.The function handles empty strings by returning empty, but names with empty parts (e.g., leading/trailing spaces after split) could produce undefined access on
n[0].💡 Proposed fix
function getInitials(name: string): string { return name .split(" ") + .filter(Boolean) .map((n) => n[0]) .join("") .toUpperCase() .slice(0, 2); }packages/ai-chat/src/stream/useCollectionData.ts (1)
47-62: Redundant ref initialization pattern.The
subscribeRefis initialized with a function on lines 47-53, then immediately overwritten on lines 56-62 with an identical function. This appears to be copied verbatim from Electric SQL but the immediate reassignment makes the initial value unnecessary.The same pattern repeats for
getSnapshotRef(lines 66-93). Since you noted this is "copied verbatim from@electric-sql/react-durable-session", you may want to keep it as-is for parity, but be aware this creates dead code on initial render.apps/web/src/app/(dashboard)/chat/[sessionId]/components/ChatRoom/ChatRoom.tsx (1)
74-74: Consider using drafts for typing indicators.
typingUsersis hardcoded as an empty array, butuseChatSessionprovidesdraftswhich could be used to show who is currently typing.apps/web/src/app/(dashboard)/chat/components/ChatMessageList/ChatMessageList.tsx (1)
40-41: Unused ref.
containerRefis assigned to the container div but never used for any functionality. Consider removing it or documenting its intended purpose.apps/streams/src/index.ts (1)
125-132: Consider adding a body size limit to prevent memory exhaustion.The
readBodyfunction accumulates chunks without any size limit. A malicious client could send an arbitrarily large body and exhaust server memory.♻️ Proposed fix with size limit
+const MAX_BODY_SIZE = 1024 * 1024; // 1MB + function readBody(req: import("node:http").IncomingMessage): Promise<string> { return new Promise((resolve, reject) => { const chunks: Buffer[] = []; - req.on("data", (chunk) => chunks.push(chunk)); + let totalSize = 0; + req.on("data", (chunk) => { + totalSize += chunk.length; + if (totalSize > MAX_BODY_SIZE) { + req.destroy(); + reject(new Error("Request body too large")); + return; + } + chunks.push(chunk); + }); req.on("end", () => resolve(Buffer.concat(chunks).toString("utf-8"))); req.on("error", reject); }); }apps/streams/src/session-registry.ts (2)
11-16: DuplicatedSessionInfointerface.This interface is also defined in
apps/mobile/lib/streams/client.tswith identical structure. Consider extracting to a shared package to maintain a single source of truth.
55-66: File write is not atomic.
writeFileSyncdirectly overwrites the file. If the process crashes mid-write, the file could be corrupted. For increased reliability, consider writing to a temp file and atomically renaming.♻️ Proposed atomic write
+import { renameSync } from "node:fs"; + private persist(): void { try { const dir = dirname(this.filePath); if (!existsSync(dir)) { mkdirSync(dir, { recursive: true }); } const data = JSON.stringify(Array.from(this.sessions.values()), null, 2); - writeFileSync(this.filePath, data, "utf-8"); + const tempPath = `${this.filePath}.tmp`; + writeFileSync(tempPath, data, "utf-8"); + renameSync(tempPath, this.filePath); } catch (error) { console.error("[session-registry] Failed to persist sessions:", error); } }apps/mobile/lib/streams/client.ts (1)
38-47: Consider adding request timeouts to prevent indefinite hangs.Network requests without timeouts can hang indefinitely on poor connections, especially problematic on mobile. Consider using
AbortControllerwith a timeout.Example timeout implementation
async function fetchWithTimeout( url: string, options: RequestInit = {}, timeoutMs = 10000 ): Promise<Response> { const controller = new AbortController(); const timeoutId = setTimeout(() => controller.abort(), timeoutMs); try { return await fetch(url, { ...options, signal: controller.signal }); } finally { clearTimeout(timeoutId); } }apps/mobile/screens/(authenticated)/chat/ChatScreen.tsx (1)
99-106: Redundant dependency in useEffect.Including both
streamingMessageandstreamingMessage?.contentin the dependency array is redundant. WhenstreamingMessagechanges (including itscontentproperty), the effect will re-run. The?.contentaccess is unnecessary as a separate dependency.Suggested fix
// Scroll to bottom when new messages arrive useEffect(() => { if (messages.length > 0 || streamingMessage) { setTimeout(() => { flatListRef.current?.scrollToEnd({ animated: true }); }, 100); } - }, [messages.length, streamingMessage?.content, streamingMessage]); + }, [messages.length, streamingMessage]);packages/ai-chat/src/stream/useChatSession.ts (1)
208-215: Consider removingclientOptions.userfrom dependency array.Static analysis correctly identifies that
clientOptions.usermay be an unnecessary dependency. The user identity typically doesn't change during a session, and including the entire options object can cause unnecessary re-computations. If user identity genuinely changes (logout/login), the component would likely remount anyway.However, if there are legitimate scenarios where user changes mid-session, you can safely ignore this lint warning.
Suggested fix if user won't change mid-session
// Current user's draft const draft = useMemo((): string => { const user = clientOptions.user; if (!user) return ""; const myDraft = draftRows.find( (d: StreamDraft) => d.userId === user.userId, ); return myDraft?.content ?? ""; - }, [draftRows, clientOptions.user]); + }, [draftRows, clientOptions.user?.userId]);Using
clientOptions.user?.userIdinstead of the entire user object provides more granular dependency tracking.packages/trpc/src/router/chat/chat.ts (1)
220-220: Avoidz.any()fortoolCalls- use a more specific schema.Per coding guidelines: "Avoid
anytype unless necessary". Usingz.any()bypasses validation entirely, which could allow malformed data into the database.Suggested improvement
- toolCalls: z.any().optional(), + toolCalls: z + .array( + z.object({ + id: z.string(), + type: z.string(), + function: z.object({ + name: z.string(), + arguments: z.string(), + }).optional(), + }) + ) + .optional(),If the exact schema isn't known, at minimum use
z.unknown()which is safer thanz.any(), orz.record(z.unknown())for object-like data.apps/desktop/src/renderer/stores/chatStore.ts (1)
19-24: Prefer a params object forrenameSession.This avoids positional confusion and matches the repo guideline.
♻️ Suggested refactor
interface ChatStore { sessions: ChatSession[]; createSession: (name?: string) => ChatSession; deleteSession: (id: string) => void; - renameSession: (id: string, name: string) => void; + renameSession: (params: { id: string; name: string }) => void; } @@ - renameSession: (id: string, name: string) => { + renameSession: ({ id, name }: { id: string; name: string }) => { set((state) => ({ sessions: state.sessions.map((s) => s.id === id ? { ...s, name, updatedAt: new Date().toISOString() } : s, ), })); },As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 76-83
apps/desktop/src/main/lib/claude/session-manager.ts (2)
18-20: Extract magic numbers for batching and polling.These constants will be easier to tune and reason about.
♻️ Suggested refactor
// Durable Stream server URL - configurable via env const DURABLE_STREAM_URL = process.env.DURABLE_STREAM_URL || "http://localhost:8080"; +const STREAM_FLUSH_BATCH_SIZE = 10; +const STREAM_FLUSH_DEBOUNCE_MS = 50; +const STREAM_POLL_INTERVAL_MS = 500; @@ - if (queue.length >= 10) { + if (queue.length >= STREAM_FLUSH_BATCH_SIZE) { this.flushEvents(sessionId); } else { - const timeout = setTimeout(() => this.flushEvents(sessionId), 50); + const timeout = setTimeout( + () => this.flushEvents(sessionId), + STREAM_FLUSH_DEBOUNCE_MS, + ); this.flushTimeouts.set(sessionId, timeout); } @@ - this.intervalId = setInterval(() => this.poll(), 500); + this.intervalId = setInterval(() => this.poll(), STREAM_POLL_INTERVAL_MS);As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Also applies to: 160-165, 226-227
446-496: Use a params object forprocessSDKMessage.This avoids positional parameter confusion and matches the repo guideline.
♻️ Suggested refactor
- this.processSDKMessage(sessionId, session, msg); + this.processSDKMessage({ sessionId, session, msg }); @@ - private processSDKMessage( - sessionId: string, - session: ActiveSession, - msg: unknown, - ): void { + private processSDKMessage({ + sessionId, + session, + msg, + }: { + sessionId: string; + session: ActiveSession; + msg: unknown; + }): void {As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
| .mutation(async ({ input }) => { | ||
| await claudeSessionManager.startSession({ | ||
| sessionId: input.sessionId, | ||
| cwd: input.cwd, | ||
| claudeSessionId: input.claudeSessionId, | ||
| }); | ||
| return { success: true }; |
There was a problem hiding this comment.
Mutations lack error handling.
The startSession, interrupt, and stopSession mutations call claudeSessionManager methods without error handling. Per coding guidelines, errors should never be silently swallowed and should at minimum be logged with context.
🔧 Proposed fix with error handling
startSession: publicProcedure
.input(
z.object({
sessionId: z.string(),
cwd: z.string(),
claudeSessionId: z.string().optional(),
}),
)
.mutation(async ({ input }) => {
+ try {
await claudeSessionManager.startSession({
sessionId: input.sessionId,
cwd: input.cwd,
claudeSessionId: input.claudeSessionId,
});
return { success: true };
+ } catch (error) {
+ console.error("[ai-chat/startSession] Failed to start session:", error);
+ throw error;
+ }
}),Apply similar patterns to interrupt and stopSession mutations.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .mutation(async ({ input }) => { | |
| await claudeSessionManager.startSession({ | |
| sessionId: input.sessionId, | |
| cwd: input.cwd, | |
| claudeSessionId: input.claudeSessionId, | |
| }); | |
| return { success: true }; | |
| .mutation(async ({ input }) => { | |
| try { | |
| await claudeSessionManager.startSession({ | |
| sessionId: input.sessionId, | |
| cwd: input.cwd, | |
| claudeSessionId: input.claudeSessionId, | |
| }); | |
| return { success: true }; | |
| } catch (error) { | |
| console.error("[ai-chat/startSession] Failed to start session:", error); | |
| throw error; | |
| } | |
| }), |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 29 - 35, The
mutations calling claudeSessionManager (the startSession, interrupt, and
stopSession mutation handlers) currently invoke async methods without error
handling; wrap each call in a try/catch, log the error with context (include
identifying data such as input.sessionId, input.claudeSessionId, input.cwd), and
surface the failure to the caller by throwing a TRPCError (e.g., code:
'INTERNAL') or returning a failure result. Update the handlers that call
claudeSessionManager.startSession, claudeSessionManager.interrupt, and
claudeSessionManager.stopSession to follow this pattern and use the existing
logger in scope (e.g., ctx.logger or processLogger) for the contextual log
message.
| interface ClaudeCredentials { | ||
| apiKey: string; | ||
| source: "env" | "config" | "keychain"; | ||
| } | ||
|
|
||
| interface ClaudeConfigFile { | ||
| apiKey?: string; | ||
| api_key?: string; | ||
| oauthAccessToken?: string; | ||
| oauth_access_token?: string; | ||
| // Claude Code CLI format | ||
| claudeAiOauth?: { | ||
| accessToken?: string; | ||
| refreshToken?: string; | ||
| expiresAt?: number; | ||
| }; | ||
| } |
There was a problem hiding this comment.
Config‑sourced API keys are currently ignored.
buildClaudeEnv skips all source: "config" credentials, even when they’re raw API keys. That contradicts the comment and prevents CLI auth for users who only configured an API key in the config file.
🐛 Suggested fix
interface ClaudeCredentials {
apiKey: string;
source: "env" | "config" | "keychain";
+ kind: "apiKey" | "oauth";
}
@@
function getCredentialsFromEnv(): ClaudeCredentials | null {
const apiKey = process.env.ANTHROPIC_API_KEY;
if (apiKey) {
- return { apiKey, source: "env" };
+ return { apiKey, source: "env", kind: "apiKey" };
}
return null;
}
@@
- // Check for Claude Code CLI OAuth format first
+ // Check for Claude Code CLI OAuth format first
if (config.claudeAiOauth?.accessToken) {
console.log(
`[claude/auth] Found OAuth credentials in: ${configPath}`,
);
- return { apiKey: config.claudeAiOauth.accessToken, source: "config" };
+ return {
+ apiKey: config.claudeAiOauth.accessToken,
+ source: "config",
+ kind: "oauth",
+ };
}
- // Fall back to other formats
- const apiKey =
- config.apiKey ||
- config.api_key ||
- config.oauthAccessToken ||
- config.oauth_access_token;
+ // Fall back to other formats
+ const oauthToken = config.oauthAccessToken || config.oauth_access_token;
+ if (oauthToken) {
+ return { apiKey: oauthToken, source: "config", kind: "oauth" };
+ }
+
+ const apiKey = config.apiKey || config.api_key;
if (apiKey) {
console.log(`[claude/auth] Found credentials in: ${configPath}`);
- return { apiKey, source: "config" };
+ return { apiKey, source: "config", kind: "apiKey" };
}
@@
if (result) {
console.log("[claude/auth] Found credentials in macOS Keychain");
- return { apiKey: result, source: "keychain" };
+ return { apiKey: result, source: "keychain", kind: "apiKey" };
}
@@
if (result) {
console.log(
"[claude/auth] Found credentials in macOS Keychain (anthropic-api-key)",
);
- return { apiKey: result, source: "keychain" };
+ return { apiKey: result, source: "keychain", kind: "apiKey" };
}
@@
- if (credentials && credentials.source !== "config") {
+ if (credentials?.kind === "apiKey") {
// Only use env or keychain credentials (not OAuth from config)
env.ANTHROPIC_API_KEY = credentials.apiKey;
console.log(`[claude/auth] Using API key from ${credentials.source}`);
}Also applies to: 47-81, 180-218
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/claude/auth.ts` around lines 15 - 31,
buildClaudeEnv currently ignores ClaudeCredentials with source: "config" even
when they contain a raw API key; update buildClaudeEnv to accept config-sourced
credentials and extract API keys from ClaudeConfigFile variants. Specifically,
inside buildClaudeEnv (and any helper that filters credentials), stop excluding
entries where credential.source === "config" and instead resolve apiKey from
ClaudeConfigFile fields (apiKey, api_key, oauthAccessToken, oauth_access_token,
or claudeAiOauth.accessToken) on the credential.config object; preserve existing
precedence (env > config > keychain) and fallback logic but ensure
config-provided raw keys are turned into the same env variables used for CLI
auth.
| // Emit message complete when stream ends | ||
| if (session.accumulatedContent) { | ||
| this.emitEvent(session, { | ||
| type: "message_complete", | ||
| sessionId, | ||
| content: session.accumulatedContent, | ||
| toolCalls: | ||
| session.toolCalls.length > 0 ? session.toolCalls : undefined, | ||
| claudeSessionId: session.claudeSessionId, | ||
| } satisfies MessageCompleteEvent); | ||
| } |
There was a problem hiding this comment.
Tool‑only assistant responses can drop message_complete + done chunks.
message_complete only fires when accumulatedContent is non‑empty, and emitEvent only posts a done chunk when a currentMessageId exists. For tool‑only responses, this can leave clients hanging.
🐛 Suggested fix
- if (session.accumulatedContent) {
+ if (session.accumulatedContent || session.toolCalls.length > 0) {
this.emitEvent(session, {
type: "message_complete",
sessionId,
content: session.accumulatedContent,
toolCalls:
session.toolCalls.length > 0 ? session.toolCalls : undefined,
claudeSessionId: session.claudeSessionId,
} satisfies MessageCompleteEvent);
}
@@
- } else if (
- event.type === "message_complete" &&
- session.currentMessageId
- ) {
+ } else if (event.type === "message_complete") {
+ if (!session.currentMessageId) {
+ session.currentMessageId = crypto.randomUUID();
+ session.currentSeq = 0;
+ }
+ if (session.currentSeq === 0) {
+ this.postChunkToStream(session, {
+ messageId: session.currentMessageId,
+ actorId: "claude",
+ role: "assistant",
+ chunk: JSON.stringify({
+ type: "whole-message",
+ content: event.content,
+ }),
+ seq: session.currentSeq++,
+ });
+ }
// Post done chunk to mark message as complete
this.postChunkToStream(session, {
messageId: session.currentMessageId,
actorId: "claude",
role: "assistant",Also applies to: 719-745
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/claude/session-manager.ts` around lines 449 - 459,
The code currently only emits a "message_complete" when
session.accumulatedContent is non-empty and relies on session.currentMessageId
inside emitEvent to produce the final "done" chunk, which can leave clients
waiting for tool-only responses; remove the accumulatedContent gate so you
always emit a MessageCompleteEvent (use empty string when there's no content)
and update the emitEvent invocation/logic so it will produce the final done
chunk even if session.currentMessageId is missing for tool-only responses (e.g.,
use session.currentMessageId || generate/assign a safe ephemeral id or adjust
emitEvent to treat tool-only message_complete events as terminal and send the
done chunk).
| // Auto-scroll to bottom when messages change | ||
| useEffect(() => { | ||
| bottomRef.current?.scrollIntoView({ behavior: "smooth" }); | ||
| }, []); |
There was a problem hiding this comment.
Auto-scroll won't trigger on new messages.
Same issue as the desktop version: the useEffect dependency array is empty, so auto-scroll only runs on mount, not when messages changes.
🐛 Proposed fix
// Auto-scroll to bottom when messages change
useEffect(() => {
bottomRef.current?.scrollIntoView({ behavior: "smooth" });
- }, []);
+ }, [messages]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // Auto-scroll to bottom when messages change | |
| useEffect(() => { | |
| bottomRef.current?.scrollIntoView({ behavior: "smooth" }); | |
| }, []); | |
| // Auto-scroll to bottom when messages change | |
| useEffect(() => { | |
| bottomRef.current?.scrollIntoView({ behavior: "smooth" }); | |
| }, [messages]); |
🤖 Prompt for AI Agents
In
`@apps/web/src/app/`(dashboard)/chat/components/ChatMessageList/ChatMessageList.tsx
around lines 43 - 46, The auto-scroll useEffect in the ChatMessageList component
only runs on mount because its dependency array is empty; update the effect that
calls bottomRef.current?.scrollIntoView(...) to depend on message changes (e.g.,
add messages or messages.length to the dependency array) so it re-runs when new
messages arrive; locate the useEffect in ChatMessageList.tsx and replace the
empty deps with an appropriate messages-based dependency to ensure scrolling on
new messages.
| function formatRelativeTime(date: Date): string { | ||
| const now = new Date(); | ||
| const diffMs = now.getTime() - date.getTime(); | ||
| const diffMins = Math.floor(diffMs / 60000); | ||
| const diffHours = Math.floor(diffMs / 3600000); | ||
| const diffDays = Math.floor(diffMs / 86400000); | ||
|
|
||
| if (diffMins < 1) return "just now"; | ||
| if (diffMins < 60) return `${diffMins}m ago`; | ||
| if (diffHours < 24) return `${diffHours}h ago`; | ||
| if (diffDays < 7) return `${diffDays}d ago`; | ||
| return date.toLocaleDateString(); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, verify the file exists and check its content
if [ -f "apps/web/src/app/(dashboard)/chat/components/ChatSessionList/ChatSessionList.tsx" ]; then
echo "=== File found ==="
wc -l "apps/web/src/app/(dashboard)/chat/components/ChatSessionList/ChatSessionList.tsx"
else
echo "=== File not found with escaped parentheses, trying alternate path ==="
fd -t f "ChatSessionList.tsx" apps/web/src 2>/dev/null | head -5
fiRepository: superset-sh/superset
Length of output: 166
🏁 Script executed:
#!/bin/bash
# Check for date-fns, dayjs, moment in package.json
echo "=== Checking for date libraries in package.json ==="
if [ -f "package.json" ]; then
jq -r '.dependencies // {} | keys[]' package.json 2>/dev/null | grep -E "date-fns|dayjs|moment" && echo "Found date library" || echo "No date-fns/dayjs/moment in dependencies"
else
echo "package.json not found in root"
fd -t f "package.json" | head -5
fiRepository: superset-sh/superset
Length of output: 156
🏁 Script executed:
#!/bin/bash
# Check for usage of date libraries in the codebase
echo "=== Searching for date library imports in TypeScript/TSX files ==="
rg -l "from ['\"]date-fns|from ['\"]dayjs|from ['\"]moment" apps/web/src --type ts --type tsx 2>/dev/null | head -10 || echo "No imports found in search"Repository: superset-sh/superset
Length of output: 130
🏁 Script executed:
#!/bin/bash
# Read the actual file content around lines 26-38 to verify the magic numbers
echo "=== Content of ChatSessionList.tsx (lines 20-45) ==="
sed -n '20,45p' "apps/web/src/app/(dashboard)/chat/components/ChatSessionList/ChatSessionList.tsx"Repository: superset-sh/superset
Length of output: 822
🏁 Script executed:
#!/bin/bash
# Check if there are other similar time formatting functions elsewhere in the project
echo "=== Searching for other formatRelativeTime or similar time formatting functions ==="
rg -n "formatRelativeTime|formatDistanceToNow|timeAgo|relative.*time" apps/web/src --type ts --type tsx -B 2 -A 5 2>/dev/null | head -30Repository: superset-sh/superset
Length of output: 147
Extract magic numbers to named constants.
The time conversion divisors (60000, 3600000, 86400000) must be extracted to named constants at module top per coding guidelines for TypeScript files.
♻️ Proposed refactor
+const MS_PER_MINUTE = 60_000;
+const MS_PER_HOUR = 3_600_000;
+const MS_PER_DAY = 86_400_000;
+
function formatRelativeTime(date: Date): string {
const now = new Date();
const diffMs = now.getTime() - date.getTime();
- const diffMins = Math.floor(diffMs / 60000);
- const diffHours = Math.floor(diffMs / 3600000);
- const diffDays = Math.floor(diffMs / 86400000);
+ const diffMins = Math.floor(diffMs / MS_PER_MINUTE);
+ const diffHours = Math.floor(diffMs / MS_PER_HOUR);
+ const diffDays = Math.floor(diffMs / MS_PER_DAY);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function formatRelativeTime(date: Date): string { | |
| const now = new Date(); | |
| const diffMs = now.getTime() - date.getTime(); | |
| const diffMins = Math.floor(diffMs / 60000); | |
| const diffHours = Math.floor(diffMs / 3600000); | |
| const diffDays = Math.floor(diffMs / 86400000); | |
| if (diffMins < 1) return "just now"; | |
| if (diffMins < 60) return `${diffMins}m ago`; | |
| if (diffHours < 24) return `${diffHours}h ago`; | |
| if (diffDays < 7) return `${diffDays}d ago`; | |
| return date.toLocaleDateString(); | |
| } | |
| const MS_PER_MINUTE = 60_000; | |
| const MS_PER_HOUR = 3_600_000; | |
| const MS_PER_DAY = 86_400_000; | |
| function formatRelativeTime(date: Date): string { | |
| const now = new Date(); | |
| const diffMs = now.getTime() - date.getTime(); | |
| const diffMins = Math.floor(diffMs / MS_PER_MINUTE); | |
| const diffHours = Math.floor(diffMs / MS_PER_HOUR); | |
| const diffDays = Math.floor(diffMs / MS_PER_DAY); | |
| if (diffMins < 1) return "just now"; | |
| if (diffMins < 60) return `${diffMins}m ago`; | |
| if (diffHours < 24) return `${diffHours}h ago`; | |
| if (diffDays < 7) return `${diffDays}d ago`; | |
| return date.toLocaleDateString(); | |
| } |
🤖 Prompt for AI Agents
In
`@apps/web/src/app/`(dashboard)/chat/components/ChatSessionList/ChatSessionList.tsx
around lines 26 - 38, The function formatRelativeTime uses magic number literals
for milliseconds (60000, 3600000, 86400000); extract these into descriptive
module-level constants (e.g., MS_PER_MINUTE, MS_PER_HOUR, MS_PER_DAY) at the top
of the file and replace the literals in formatRelativeTime with those constants
so the conversions read MS_PER_MINUTE, MS_PER_HOUR, and MS_PER_DAY respectively,
keeping the logic unchanged.
| export async function createStream( | ||
| baseUrl: string, | ||
| sessionId: string, | ||
| ): Promise<boolean> { | ||
| const response = await fetch(`${baseUrl}/streams/${sessionId}`, { | ||
| method: "PUT", | ||
| headers: { "Content-Type": "application/json" }, | ||
| }); | ||
|
|
||
| // 201 = created, 200 = already exists | ||
| return response.status === 201; | ||
| } |
There was a problem hiding this comment.
Missing error handling and should use named parameters.
- The function silently treats any non-201 response (including errors like 500) as "already exists"
- Per coding guidelines, functions with 2+ parameters should use a params object
Proposed fix
-export async function createStream(
- baseUrl: string,
- sessionId: string,
-): Promise<boolean> {
- const response = await fetch(`${baseUrl}/streams/${sessionId}`, {
- method: "PUT",
- headers: { "Content-Type": "application/json" },
- });
-
- // 201 = created, 200 = already exists
- return response.status === 201;
+export async function createStream({
+ baseUrl,
+ sessionId,
+}: {
+ baseUrl: string;
+ sessionId: string;
+}): Promise<boolean> {
+ const response = await fetch(`${baseUrl}/streams/${sessionId}`, {
+ method: "PUT",
+ headers: { "Content-Type": "application/json" },
+ });
+
+ if (!response.ok && response.status !== 200) {
+ console.error(`[ai-chat/stream] Failed to create stream: ${response.status}`);
+ throw new Error(`Failed to create stream: ${response.status}`);
+ }
+
+ // 201 = created, 200 = already exists
+ return response.status === 201;
}🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/actions.ts` around lines 17 - 28, Update
createStream to accept a single params object (e.g., createStream({ baseUrl,
sessionId }): Promise<boolean>) and add proper error handling: perform the PUT
as before, return true only for 201, return false for the known "already exists"
success code (200 or explicit 409 if your API uses that), and for any other
non-2xx response throw an Error that includes the response.status and response
text/body; also catch and rethrow network/fetch errors with a clear message so
callers can distinguish server errors from "already exists". Use the function
name createStream and parameter names baseUrl and sessionId to locate the code
to change.
| export const sessionStateSchema = createStateSchema({ | ||
| chunks: { | ||
| schema: chunkSchema, | ||
| type: "chunk", | ||
| primaryKey: "id", | ||
| }, |
There was a problem hiding this comment.
Primary key "id" doesn't exist in chunkSchema.
The chunks collection specifies primaryKey: "id", but chunkSchema defines only: messageId, actorId, role, chunk, seq, createdAt. This will likely cause runtime errors when the state management library attempts to key by a non-existent field.
Proposed fix - add id field or use messageId
Option 1: Add id field to schema:
export const chunkSchema = z.object({
+ id: z.string(),
messageId: z.string(),
actorId: z.string(),
role: z.enum(["user", "assistant", "system"]),
chunk: z.string(),
seq: z.number(),
createdAt: z.string(),
});Option 2: Use existing field as primary key (if messageId+seq is unique):
chunks: {
schema: chunkSchema,
type: "chunk",
- primaryKey: "id",
+ primaryKey: "messageId", // or consider composite key if supported
},📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const sessionStateSchema = createStateSchema({ | |
| chunks: { | |
| schema: chunkSchema, | |
| type: "chunk", | |
| primaryKey: "id", | |
| }, | |
| export const sessionStateSchema = createStateSchema({ | |
| chunks: { | |
| schema: chunkSchema, | |
| type: "chunk", | |
| primaryKey: "messageId", | |
| }, |
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/schema.ts` around lines 64 - 69, The chunks
collection in sessionStateSchema declares primaryKey: "id" but chunkSchema has
no id field (only messageId, actorId, role, chunk, seq, createdAt), so update
the schema: either add a unique id field to chunkSchema (e.g., id) and ensure
it’s populated when creating chunks, or change
sessionStateSchema.chunks.primaryKey to use an existing unique field (e.g.,
"messageId" or a supported composite key like ["messageId","seq"]) so the
primary key matches an actual field defined on chunkSchema; adjust any
creation/lookup code that relies on the primary key (functions that write/read
chunks) to use the chosen key.
| // Verify session exists and current user has owner/editor access | ||
| const [session] = await db | ||
| .select() | ||
| .from(chatSessions) | ||
| .where( | ||
| and( | ||
| eq(chatSessions.id, input.sessionId), | ||
| eq(chatSessions.organizationId, organizationId), | ||
| ), | ||
| ) | ||
| .limit(1); | ||
|
|
||
| if (!session) { | ||
| throw new TRPCError({ | ||
| code: "NOT_FOUND", | ||
| message: "Chat session not found", | ||
| }); | ||
| } |
There was a problem hiding this comment.
Missing authorization check for adding participants.
The comment on line 498 states "Verify session exists and current user has owner/editor access", but the implementation only verifies the session exists within the organization. Any authenticated user in the organization can add participants to any session, which may be overly permissive.
Consider checking that the current user is a participant with owner or editor role before allowing them to add others.
Suggested fix
// Verify session exists and current user has owner/editor access
const [session] = await db
.select()
.from(chatSessions)
.where(
and(
eq(chatSessions.id, input.sessionId),
eq(chatSessions.organizationId, organizationId),
),
)
.limit(1);
if (!session) {
throw new TRPCError({
code: "NOT_FOUND",
message: "Chat session not found",
});
}
+ // Verify current user has owner/editor role
+ const [currentParticipant] = await db
+ .select()
+ .from(chatParticipants)
+ .where(
+ and(
+ eq(chatParticipants.sessionId, input.sessionId),
+ eq(chatParticipants.userId, ctx.session.user.id),
+ ),
+ )
+ .limit(1);
+
+ if (!currentParticipant || !["owner", "editor"].includes(currentParticipant.role)) {
+ throw new TRPCError({
+ code: "FORBIDDEN",
+ message: "You do not have permission to add participants",
+ });
+ }🤖 Prompt for AI Agents
In `@packages/trpc/src/router/chat/chat.ts` around lines 498 - 515, The current
code only checks that the session exists in the organization but does not verify
the caller's role; update the handler after the chatSessions query to ensure the
authenticated user (current user id) is a participant on the session with role
"owner" or "editor" before allowing adding participants. Query the
session-participants table (e.g., chatSessionParticipants) for a row matching
input.sessionId and the current user's id and assert participant.role is "owner"
or "editor"; if not, throw a TRPCError with code "FORBIDDEN" and a descriptive
message. Ensure you reference the existing variables session, input.sessionId
and organizationId and use the same db instance for the additional query.
saddlepaddle
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@apps/desktop/scripts/download-claude-binary.ts`:
- Around line 74-76: Replace the inline assignment expression in the response
data event handlers with explicit block bodies to satisfy Biome lint: in the
res.on("data", (chunk) => (data += chunk)) handler (and the similar handler
around lines 103-105), change the arrow callback to use a block body that
performs data += chunk on its own line, and ensure the res.on("end", () =>
resolve(JSON.parse(data) as T)) remains a block or single-statement arrow as
appropriate; update the handlers in the same function scope where variables
data, res, and resolve are defined.
In `@apps/desktop/src/main/lib/claude/auth.ts`:
- Around line 102-135: The two empty catch blocks around the execSync keychain
lookups (the calls using 'security find-generic-password -s "claude-cli" -a
"api-key" -w ...' and 'security find-generic-password -s "anthropic-api-key" -w
...') are swallowing errors; update each catch to log the error with context
(e.g., console.warn or the module logger) including which keychain query failed
and the error object/message (prefixed like "[claude/auth] keychain lookup
failed: ..."), and apply the same change to the similar catch at lines 228-235
so failures are not silently ignored.
In `@apps/desktop/src/main/lib/claude/binary.ts`:
- Around line 48-60: The getClaudeBinaryPath function declares it may return
null but always returns a string; either update the contract to always return a
string by changing the signature to string and updating the doc comment, or
implement a PATH existence check before returning "claude" (e.g., use a
cross-platform check like which/where via a small helper or fs+child_process)
and return null when not found; modify getClaudeBinaryPath, and use
hasBundledClaudeBinary and getBundledClaudeBinaryPath to locate the bundled
binary as currently done, then add the PATH check fallback or adjust the return
type/docs accordingly so the implementation and signature match.
In `@apps/desktop/src/main/lib/claude/session-manager.ts`:
- Around line 151-153: The empty catch in the stream polling block should not
swallow errors; replace it with a catch that logs the error and minimal context.
Change the catch to accept the error (e.g. catch (err)) and call the module's
logger (or console) to emit a message such as "Stream poll error" plus the error
object and any available identifiers (e.g. sessionId, conversationId, or attempt
counter) from the surrounding scope; update the catch in the SessionManager
polling function in apps/desktop/src/main/lib/claude/session-manager.ts so it
logs the error with that prefix instead of doing nothing.
In `@apps/desktop/src/renderer/env.renderer.ts`:
- Line 20: The NEXT_PUBLIC_STREAMS_URL schema entry currently uses
.default("http://localhost:8080") which causes production builds to always be
truthy and attempt localhost; update the schema entry for
NEXT_PUBLIC_STREAMS_URL to either use z.url().optional() (if Durable Streams is
optional) or replace the default with a real production URL (e.g.,
"https://streams.superset.sh") so the defensive check in chatStore.ts (the if
(!streamsUrl) return; guard) works as intended.
In `@apps/desktop/src/renderer/screens/chat/ChatView.tsx`:
- Line 20: STREAM_SERVER_URL is hard-coded to "http://localhost:8080" in
ChatView.tsx which fails in packaged/shared environments; change it to read from
a configuration source (environment variable or app settings) with a safe
default. Replace the literal with a lookup that first tries your app config
(e.g., process.env.STREAM_SERVER_URL or import.meta.env.VITE_STREAM_SERVER_URL
or the app settings accessor used elsewhere in the codebase) and fall back to
"http://localhost:8080"; update any tests or callers that reference
STREAM_SERVER_URL accordingly so the value is configurable at runtime.
In
`@apps/desktop/src/renderer/screens/chat/components/ChatMessage/ChatMessage.tsx`:
- Around line 71-109: The list rendering in contentBlocks uses array index as
React keys causing remounts; update the key logic so keys are stable: for any
block with block.id (e.g., tool_use / ToolCallPart) use block.id; for text
blocks (ReactMarkdown) derive a stable key from the text content (e.g.,
`text:${block.text}` or a short hash) and for thinking blocks (ThinkingBlock)
derive a stable key from the thinking content (e.g.,
`thinking:${block.thinking}`); ensure keys are unique across block types and
prefer block.id when present.
🧹 Nitpick comments (5)
apps/api/src/app/api/electric/[...path]/utils.ts (1)
75-99: Avoid loading all session IDs into memory; prefer a subquery/join.
For orgs with many sessions, building a largeIN (...)list can hit parameter limits and add memory/latency. Consider using a subquery so the DB handles the filtering.♻️ Proposed refactor (verify Drizzle subquery support)
- // Filter participants by sessions the user's org owns - const orgSessions = await db.query.chatSessions.findMany({ - where: eq(chatSessions.organizationId, organizationId), - columns: { id: true }, - }); - - if (orgSessions.length === 0) { - return { fragment: "1 = 0", params: [] }; - } - - const sessionIds = orgSessions.map((s) => s.id); - const whereExpr = inArray( - sql`${sql.identifier(chatParticipants.sessionId.name)}`, - sessionIds, - ); + // Filter participants by sessions the user's org owns (DB-side subquery) + const orgSessionIds = db + .select({ id: chatSessions.id }) + .from(chatSessions) + .where(eq(chatSessions.organizationId, organizationId)); + const whereExpr = inArray( + sql`${sql.identifier(chatParticipants.sessionId.name)}`, + orgSessionIds, + ); const qb = new QueryBuilder(); const { sql: query, params } = qb .select() .from(chatParticipants) .where(whereExpr) .toSQL();Please verify the Drizzle API for
inArray+ subquery support in your version and adjust accordingly.apps/desktop/scripts/download-claude-binary.ts (2)
117-118: Prefer params objects for multi-arg helpers.
downloadFile(url, destPath)anddownloadPlatform(version, platformKey, manifest)violate the params-object guideline; consider{ url, destPath }and{ version, platformKey, manifest }instead.As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 224-228
206-214: Add[domain/operation]prefixes to script logs.Console logs here don’t use the required prefix format. Consider a shared
log()wrapper or aLOG_PREFIXconstant to enforce[claude/download] ….As per coding guidelines, Use prefixed console logging with pattern
[domain/operation] messagefor all logging.Also applies to: 252-255, 295-305
apps/desktop/src/main/lib/claude/session-manager.ts (1)
113-117: Extract poll interval to a named constant.
500is a magic number; lift it toSTREAM_POLL_INTERVAL_MSfor clarity and reuse.As per coding guidelines, Avoid magic numbers by extracting them to named constants at module top.
apps/desktop/src/renderer/screens/chat/components/ChatMessageList/ChatMessageList.tsx (1)
20-72: No actual key collision risk exists in the current implementation.All
Messageobjects have requiredidfields, so thekey={message.id || index}fallback never executes. TheStreamingMessageuses a statickey="streaming", and only one streaming message exists at a time per the conditional inChatView.tsx(line 76). Adding anidtoStreamingMessagewould improve consistency and code robustness for future changes, but the current code handles keys safely.
| try { | ||
| // Claude CLI stores credentials in the keychain with this service/account | ||
| const result = execSync( | ||
| 'security find-generic-password -s "claude-cli" -a "api-key" -w 2>/dev/null', | ||
| { encoding: "utf-8" }, | ||
| ).trim(); | ||
|
|
||
| if (result) { | ||
| console.log("[claude/auth] Found credentials in macOS Keychain"); | ||
| return { apiKey: result, source: "keychain" }; | ||
| } | ||
| } catch { | ||
| // Not found in keychain, this is fine | ||
| } | ||
|
|
||
| // Try alternate keychain entry format | ||
| try { | ||
| const result = execSync( | ||
| 'security find-generic-password -s "anthropic-api-key" -w 2>/dev/null', | ||
| { encoding: "utf-8" }, | ||
| ).trim(); | ||
|
|
||
| if (result) { | ||
| console.log( | ||
| "[claude/auth] Found credentials in macOS Keychain (anthropic-api-key)", | ||
| ); | ||
| return { apiKey: result, source: "keychain" }; | ||
| } | ||
| } catch { | ||
| // Not found in keychain, this is fine | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Log swallowed errors instead of dropping them.
The empty catch blocks hide failures (even if expected). Add prefixed debug/warn logs for diagnostics.
🛠️ Suggested fix
} catch {
- // Not found in keychain, this is fine
+ // Not found in keychain, this is fine
+ console.debug(
+ "[claude/auth] Keychain lookup failed for claude-cli api-key",
+ error,
+ );
}
@@
} catch {
- // Not found in keychain, this is fine
+ // Not found in keychain, this is fine
+ console.debug(
+ "[claude/auth] Keychain lookup failed for anthropic-api-key",
+ error,
+ );
}
@@
- } catch {
- return false;
- }
+ } catch (error) {
+ console.warn(
+ "[claude/auth] Failed to parse OAuth credentials file",
+ error,
+ );
+ return false;
+ }As per coding guidelines "Never swallow errors silently; at minimum log them with context".
Also applies to: 228-235
🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/claude/auth.ts` around lines 102 - 135, The two
empty catch blocks around the execSync keychain lookups (the calls using
'security find-generic-password -s "claude-cli" -a "api-key" -w ...' and
'security find-generic-password -s "anthropic-api-key" -w ...') are swallowing
errors; update each catch to log the error with context (e.g., console.warn or
the module logger) including which keychain query failed and the error
object/message (prefixed like "[claude/auth] keychain lookup failed: ..."), and
apply the same change to the similar catch at lines 228-235 so failures are not
silently ignored.
| /** | ||
| * Get the Claude binary path, falling back to PATH if bundled binary doesn't exist. | ||
| * Returns null if no binary is available. | ||
| */ | ||
| export function getClaudeBinaryPath(): string | null { | ||
| // First try bundled binary | ||
| if (hasBundledClaudeBinary()) { | ||
| return getBundledClaudeBinaryPath(); | ||
| } | ||
|
|
||
| // Fall back to checking if claude is in PATH (for development) | ||
| // We don't resolve it here - just return "claude" and let the shell find it | ||
| return "claude"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/desktop/src/main/lib/claude/binary.tsRepository: superset-sh/superset
Length of output: 2235
🏁 Script executed:
rg "getClaudeBinaryPath" --type ts --type tsx -B 2 -A 2Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
rg "getClaudeBinaryPath" -B 2 -A 2Repository: superset-sh/superset
Length of output: 1632
🏁 Script executed:
sed -n '1,50p' apps/desktop/src/main/lib/claude/session-manager.ts | head -40Repository: superset-sh/superset
Length of output: 1310
🏁 Script executed:
sed -n '35,80p' apps/desktop/src/main/lib/claude/session-manager.tsRepository: superset-sh/superset
Length of output: 1280
🏁 Script executed:
rg -n "getClaudeBinaryPath\(\)" apps/desktop/src/main/lib/claude/session-manager.ts -A 5Repository: superset-sh/superset
Length of output: 248
Remove null from contract or add PATH check.
The function signature and documentation promise null as a return value, but the implementation always returns a string. Either remove string | null and update the documentation to reflect fallback behavior, or implement an actual PATH existence check before returning "claude".
Option 1: Update type and doc to match current behavior
- * Returns null if no binary is available.
+ * Returns "claude" to rely on PATH if bundled binary doesn't exist.
*/
-export function getClaudeBinaryPath(): string | null {
+export function getClaudeBinaryPath(): string {🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/claude/binary.ts` around lines 48 - 60, The
getClaudeBinaryPath function declares it may return null but always returns a
string; either update the contract to always return a string by changing the
signature to string and updating the doc comment, or implement a PATH existence
check before returning "claude" (e.g., use a cross-platform check like
which/where via a small helper or fs+child_process) and return null when not
found; modify getClaudeBinaryPath, and use hasBundledClaudeBinary and
getBundledClaudeBinaryPath to locate the bundled binary as currently done, then
add the PATH check fallback or adjust the return type/docs accordingly so the
implementation and signature match.
| } catch { | ||
| // Ignore poll errors | ||
| } |
There was a problem hiding this comment.
Don’t swallow stream polling errors; log with context.
The empty catch makes failures invisible and complicates support. At minimum, log the error with a prefix.
Proposed fix
- } catch {
- // Ignore poll errors
- }
+ } catch (error) {
+ console.warn(
+ `[stream-watcher/poll] Failed to fetch stream ${this.sessionId}:`,
+ error,
+ );
+ }🤖 Prompt for AI Agents
In `@apps/desktop/src/main/lib/claude/session-manager.ts` around lines 151 - 153,
The empty catch in the stream polling block should not swallow errors; replace
it with a catch that logs the error and minimal context. Change the catch to
accept the error (e.g. catch (err)) and call the module's logger (or console) to
emit a message such as "Stream poll error" plus the error object and any
available identifiers (e.g. sessionId, conversationId, or attempt counter) from
the surrounding scope; update the catch in the SessionManager polling function
in apps/desktop/src/main/lib/claude/session-manager.ts so it logs the error with
that prefix instead of doing nothing.
| .default("development"), | ||
| NEXT_PUBLIC_API_URL: z.url().default("https://api.superset.sh"), | ||
| NEXT_PUBLIC_WEB_URL: z.url().default("https://app.superset.sh"), | ||
| NEXT_PUBLIC_STREAMS_URL: z.url().default("http://localhost:8080"), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's see the full context of the env.renderer.ts file
cat -n apps/desktop/src/renderer/env.renderer.tsRepository: superset-sh/superset
Length of output: 2482
🏁 Script executed:
# Search for all uses of NEXT_PUBLIC_STREAMS_URL in the codebase
rg "NEXT_PUBLIC_STREAMS_URL" -A 2 -B 2Repository: superset-sh/superset
Length of output: 1435
🏁 Script executed:
# Search for streams-related configuration or documentation
rg -i "streams" apps/desktop --type ts --type tsx -l | head -20Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check if there are environment files or deployment configs
fd -e env -e env.example -e env.production -e .env.example -e .env.productionRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Look for any comments or documentation about streams feature
rg "streams" --type md -iRepository: superset-sh/superset
Length of output: 3275
🏁 Script executed:
# Look at the syncToStreamsServer function and surrounding context
cat -n apps/desktop/src/renderer/stores/chatStore.ts | grep -A 15 "syncToStreamsServer"Repository: superset-sh/superset
Length of output: 1103
Fix: Production builds will attempt localhost connections due to incorrect default.
NEXT_PUBLIC_STREAMS_URL defaults to http://localhost:8080, but like other URL vars in the schema (NEXT_PUBLIC_API_URL, NEXT_PUBLIC_WEB_URL), it should default to a production URL or be marked optional.
The defensive check in chatStore.ts (if (!streamsUrl) return;) will not protect against this because the schema's .default() ensures the value is always truthy. In production without an explicit NEXT_PUBLIC_STREAMS_URL env var, the app will attempt to connect to localhost and fail.
Either:
- Change to
z.url().optional()if Durable Streams is an optional feature - Provide a production default URL (e.g.,
https://streams.superset.sh)
🤖 Prompt for AI Agents
In `@apps/desktop/src/renderer/env.renderer.ts` at line 20, The
NEXT_PUBLIC_STREAMS_URL schema entry currently uses
.default("http://localhost:8080") which causes production builds to always be
truthy and attempt localhost; update the schema entry for
NEXT_PUBLIC_STREAMS_URL to either use z.url().optional() (if Durable Streams is
optional) or replace the default with a real production URL (e.g.,
"https://streams.superset.sh") so the defensive check in chatStore.ts (the if
(!streamsUrl) return; guard) works as intended.
| return ( | ||
| <div className="space-y-3"> | ||
| {contentBlocks.map((block, index) => { | ||
| switch (block.type) { | ||
| case "text": | ||
| return ( | ||
| <div | ||
| key={index} | ||
| className="prose prose-sm dark:prose-invert max-w-none" | ||
| > | ||
| <ReactMarkdown | ||
| remarkPlugins={[remarkGfm]} | ||
| rehypePlugins={[rehypeRaw, rehypeSanitize]} | ||
| > | ||
| {block.text} | ||
| </ReactMarkdown> | ||
| </div> | ||
| ); | ||
| case "tool_use": | ||
| return ( | ||
| <ToolCallPart | ||
| key={block.id} | ||
| block={block} | ||
| result={toolResults?.get(block.id)} | ||
| /> | ||
| ); | ||
| case "thinking": | ||
| return <ThinkingBlock key={index} thinking={block.thinking} />; | ||
| default: | ||
| return ( | ||
| <div | ||
| key={index} | ||
| className="rounded border border-border bg-muted/30 p-2 text-xs text-muted-foreground" | ||
| > | ||
| <span className="font-mono">{block.type}</span> block | ||
| </div> | ||
| ); | ||
| } | ||
| })} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find apps/desktop/src/renderer -name "ChatMessage.tsx" -type fRepository: superset-sh/superset
Length of output: 141
🏁 Script executed:
cat -n apps/desktop/src/renderer/screens/chat/components/ChatMessage/ChatMessage.tsx | head -150Repository: superset-sh/superset
Length of output: 4799
🏁 Script executed:
rg "type BetaContentBlock|interface BetaContentBlock" -A 20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find . -name "stream*" -type f | grep -E "\.(ts|tsx)$" | head -20Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
find . -path "*ai-chat*" -name "*.ts" -o -path "*ai-chat*" -name "*.tsx" | head -20Repository: superset-sh/superset
Length of output: 755
🏁 Script executed:
cat -n packages/ai-chat/src/stream/schema.tsRepository: superset-sh/superset
Length of output: 2676
🏁 Script executed:
cat -n packages/ai-chat/src/types.tsRepository: superset-sh/superset
Length of output: 2877
🏁 Script executed:
cat -n packages/ai-chat/src/stream/index.tsRepository: superset-sh/superset
Length of output: 1212
🏁 Script executed:
rg "BetaContentBlock" -B 2 -A 5 --max-count=20Repository: superset-sh/superset
Length of output: 8085
🏁 Script executed:
cat -n packages/ai-chat/src/stream/materialize.ts | head -200Repository: superset-sh/superset
Length of output: 7360
🏁 Script executed:
cat -n packages/ai-chat/src/stream/materialize.ts | tail -n +200Repository: superset-sh/superset
Length of output: 4528
🏁 Script executed:
web_search
https://sdk.anthropic.com API documentation for BetaContentBlock typesRepository: superset-sh/superset
Length of output: 186
🌐 Web query:
Anthropic SDK BetaContentBlock types documentation
💡 Result:
In Anthropic’s SDKs, BetaContentBlock is the beta-namespace equivalent of ContentBlock, and it’s a union type whose variants represent the different “content block” shapes you can send/receive when using client.beta.* features. The most direct “documentation” is in the SDK type definitions themselves, plus Anthropic’s API docs about the beta namespace.
Where to look (official + SDK type references)
- Anthropic API docs: Beta namespace in SDKs (explains using
client.beta...+betas=[...]to enable beta features) [3] - Python SDK beta feature/type overview (summarizes
BetaContentBlockand lists the beta-only content block types) [1] - Go SDK godocs (explicitly enumerates the
BetaContentBlockUnionvariants) [2]
What “BetaContentBlock” includes (examples of variants)
From the SDK type docs, BetaContentBlock/BetaContentBlockUnion includes blocks such as:
BetaTextBlock,BetaThinkingBlock,BetaRedactedThinkingBlockBetaToolUseBlock,BetaServerToolUseBlock- Tool-result blocks for beta tools like web fetch/search and code execution
- MCP blocks like
BetaMCPToolUseBlock/BetaMCPToolResultBlock BetaContainerUploadBlock[2]
And the Python beta-type overview highlights beta content block types such as MCP tool use/result, code execution output (including bash), web search result, web fetch, and container upload blocks [1].
🏁 Script executed:
rg "BetaTextBlock|BetaToolUseBlock|BetaThinkingBlock" -A 3 | head -60Repository: superset-sh/superset
Length of output: 2789
🏁 Script executed:
find . -name "biome.json*" -o -name ".biomeignore" | head -5Repository: superset-sh/superset
Length of output: 77
🏁 Script executed:
cat -n biome.jsoncRepository: superset-sh/superset
Length of output: 762
Replace array index keys in content blocks.
Biome flags index keys in list renders; they cause incorrect remounts when blocks stream or reorder. Use stable keys derived from block identity.
For tool_use blocks, use the existing block.id. For text and thinking blocks without ids, derive stable keys from their content.
♻️ Suggested fix
+function getContentBlockKey(block: BetaContentBlock): string {
+ if ("id" in block && block.id) {
+ return block.id;
+ }
+ if (block.type === "text") {
+ return `text-${block.text}`;
+ }
+ if (block.type === "thinking") {
+ return `thinking-${block.thinking}`;
+ }
+ return `unknown-${block.type}`;
+}
+
function AssistantContent({
content,
contentBlocks,
toolResults,
}: {
@@
- <div className="space-y-3">
- {contentBlocks.map((block, index) => {
+ <div className="space-y-3">
+ {contentBlocks.map((block) => {
+ const key = getContentBlockKey(block);
switch (block.type) {
case "text":
return (
<div
- key={index}
+ key={key}
className="prose prose-sm dark:prose-invert max-w-none"
>
@@
case "tool_use":
return (
<ToolCallPart
- key={block.id}
+ key={key}
block={block}
result={toolResults?.get(block.id)}
/>
);
case "thinking":
- return <ThinkingBlock key={index} thinking={block.thinking} />;
+ return <ThinkingBlock key={key} thinking={block.thinking} />;
default:
return (
<div
- key={index}
+ key={key}
className="rounded border border-border bg-muted/30 p-2 text-xs text-muted-foreground"
>📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return ( | |
| <div className="space-y-3"> | |
| {contentBlocks.map((block, index) => { | |
| switch (block.type) { | |
| case "text": | |
| return ( | |
| <div | |
| key={index} | |
| className="prose prose-sm dark:prose-invert max-w-none" | |
| > | |
| <ReactMarkdown | |
| remarkPlugins={[remarkGfm]} | |
| rehypePlugins={[rehypeRaw, rehypeSanitize]} | |
| > | |
| {block.text} | |
| </ReactMarkdown> | |
| </div> | |
| ); | |
| case "tool_use": | |
| return ( | |
| <ToolCallPart | |
| key={block.id} | |
| block={block} | |
| result={toolResults?.get(block.id)} | |
| /> | |
| ); | |
| case "thinking": | |
| return <ThinkingBlock key={index} thinking={block.thinking} />; | |
| default: | |
| return ( | |
| <div | |
| key={index} | |
| className="rounded border border-border bg-muted/30 p-2 text-xs text-muted-foreground" | |
| > | |
| <span className="font-mono">{block.type}</span> block | |
| </div> | |
| ); | |
| } | |
| })} | |
| function getContentBlockKey(block: BetaContentBlock): string { | |
| if ("id" in block && block.id) { | |
| return block.id; | |
| } | |
| if (block.type === "text") { | |
| return `text-${block.text}`; | |
| } | |
| if (block.type === "thinking") { | |
| return `thinking-${block.thinking}`; | |
| } | |
| return `unknown-${block.type}`; | |
| } | |
| function AssistantContent({ | |
| content, | |
| contentBlocks, | |
| toolResults, | |
| }: { | |
| return ( | |
| <div className="space-y-3"> | |
| {contentBlocks.map((block) => { | |
| const key = getContentBlockKey(block); | |
| switch (block.type) { | |
| case "text": | |
| return ( | |
| <div | |
| key={key} | |
| className="prose prose-sm dark:prose-invert max-w-none" | |
| > | |
| <ReactMarkdown | |
| remarkPlugins={[remarkGfm]} | |
| rehypePlugins={[rehypeRaw, rehypeSanitize]} | |
| > | |
| {block.text} | |
| </ReactMarkdown> | |
| </div> | |
| ); | |
| case "tool_use": | |
| return ( | |
| <ToolCallPart | |
| key={key} | |
| block={block} | |
| result={toolResults?.get(block.id)} | |
| /> | |
| ); | |
| case "thinking": | |
| return <ThinkingBlock key={key} thinking={block.thinking} />; | |
| default: | |
| return ( | |
| <div | |
| key={key} | |
| className="rounded border border-border bg-muted/30 p-2 text-xs text-muted-foreground" | |
| > | |
| <span className="font-mono">{block.type}</span> block | |
| </div> | |
| ); | |
| } | |
| })} | |
| </div> | |
| ); | |
| } |
🧰 Tools
🪛 Biome (2.3.13)
[error] 78-78: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 98-98: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
[error] 102-102: Avoid using the index of an array as key property in an element.
This is the source of the key value.
The order of the items may change, and this also affects performances and component state.
Check the React documentation.
(lint/suspicious/noArrayIndexKey)
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/chat/components/ChatMessage/ChatMessage.tsx`
around lines 71 - 109, The list rendering in contentBlocks uses array index as
React keys causing remounts; update the key logic so keys are stable: for any
block with block.id (e.g., tool_use / ToolCallPart) use block.id; for text
blocks (ReactMarkdown) derive a stable key from the text content (e.g.,
`text:${block.text}` or a short hash) and for thinking blocks (ThinkingBlock)
derive a stable key from the thinking content (e.g.,
`thinking:${block.thinking}`); ensure keys are unique across block types and
prefer block.id when present.
- Add session registry API to streams server (GET/POST /sessions) - Create mobile chat screens (ChatListScreen, ChatScreen) - Add mobile streams client with RN-compatible UUID generation - Consolidate message flow: all clients POST to stream, desktop watches - Add StreamWatcher to session-manager that polls for new user messages - Desktop ChatView now uses hook's sendMessage directly - Remove legacy tRPC sendMessage procedure - Sync desktop sessions to streams server for mobile access
- Subscription no longer emits on subscribe (hydration uses query) - Clear old session state before setting new token on re-auth - Add documentation for auth events - Bump TanStack dependencies - Gitignore streams data files
…ering
Remove custom envelope wrapping from durable stream chunks. SDK messages
are now stored as raw passthrough with minimal ordering metadata (createdAt,
_seq). Turn detection moved from session manager to client-side materialize.
Key changes:
- schema: chunk schema is now a loose record (no messageId/seq/role envelope)
- session-manager: pure passthrough of SDK messages with createdAt + _seq
- materialize: full rewrite — sorts chunks by time, detects turn boundaries
from SDK type transitions, builds contentBlocks from stream events
- client/mobile: user input uses { type: "user_input" } format
- useChatSession: simplified — delegates all grouping to materializeMessages
- ToolCallPart: inline display (no collapsible), always shows input/output
- ChatMessage: removed animate-pulse on streaming messages
Move ChatScreen and its components into screens/(authenticated)/chat/[sessionId]/ to mirror the app/ route structure per mobile AGENTS.md guidelines. Also fix NodeJS.Timeout type in desktop session-manager.
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Fix all issues with AI agents
In @.env.example:
- Around line 33-34: Reorder the two environment variable entries so
EXPO_PUBLIC_STREAMS_URL appears before NEXT_PUBLIC_STREAMS_URL in the
.env.example file to satisfy dotenv-linter; specifically swap the lines
containing EXPO_PUBLIC_STREAMS_URL and NEXT_PUBLIC_STREAMS_URL so the key
EXPO_PUBLIC_STREAMS_URL comes first.
In `@apps/desktop/electron-builder.ts`:
- Around line 59-64: The extraResources.from currently uses
"resources/bin/${platform}-${arch}" which is invalid for electron-builder and
mismatches the download script's Node-style dirs (e.g., darwin-arm64, linux-x64,
win32-x64); update the config referenced by extraResources.from to point to the
actual directory names created by the download script (or change the download
script to emit electron-builder names). Concretely, replace the single-template
from with platform-aware logic: map electron-builder ${os}/${arch} to the
download-script names (or use process.platform mapping such as darwin -> darwin,
win32 -> win32, linux -> linux and include arch) so the path used by
extraResources.from matches the real folders (or alternatively change the
downloader to write mac/linux/win folders to match electron-builder). Ensure the
path string used by extraResources.from exactly matches the download script's
output naming convention.
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`:
- Around line 217-224: If durable stream initialization fails in the session
creation path, do not swallow the error; instead throw a TRPCError so the
mutation fails and the client is not misled. In the block where
enableDurableStream is checked and createProducer(sessionId) is awaited, catch
the error and rethrow a TRPCError (e.g., new TRPCError({ code: 'INTERNAL',
message: 'Failed to initialize durable stream', cause: error })) so the session
creation aborts; reference createProducer, enableDurableStream, and sessionId in
your change and import TRPCError from `@trpc/server` if not already imported.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/chats/`$chatId/page.tsx:
- Around line 23-37: The calls to createStream in useEffect, handleCreateChat,
and handleSelectChat currently swallow or log errors without context and may
produce unhandled rejections; wrap each createStream invocation in a try/catch
(or attach a .catch) and log errors using the required prefixed format (e.g.,
"[chats/createStream] Failed to create stream for chatId=<id>: <error>") so that
useEffect, handleCreateChat (before navigate) and handleSelectChat all log
contextual, prefixed messages and do not allow unhandled promise rejections;
reference createStream, STREAM_SERVER_URL, useEffect, handleCreateChat,
handleSelectChat, navigate, and createSession when making these changes.
In `@apps/streams/src/index.ts`:
- Around line 125-132: The readBody function currently accumulates request
chunks without a size limit, allowing unbounded memory use; update readBody(req:
IncomingMessage) to enforce a configurable maxBytes (e.g., 1MB) by tracking
totalLength as chunks arrive, and if totalLength exceeds maxBytes immediately
remove listeners, call reject with a clear error (e.g., PayloadTooLargeError or
a custom Error) and destroy the request stream; ensure the normal "end" and
"error" handlers still resolve/reject correctly and that listeners are cleaned
up on all paths so no memory leaks remain.
In `@apps/streams/src/session-registry.ts`:
- Around line 68-83: The load() method currently parses file JSON and assumes it
matches SessionInfo[]; add validation to avoid corrupt data propagation by
validating the parsed value before inserting into this.sessions: in load() after
JSON.parse(data) validate that the top-level value is an array and each element
conforms to the SessionInfo shape (required fields like sessionId, userId,
createdAt, etc.) using a schema validator or a small type-guard function, skip
or filter out invalid entries, log warnings for any malformed items, and only
call this.sessions.set(session.sessionId, session) for validated SessionInfo
objects so downstream code using SessionInfo is safe.
In `@packages/ai-chat/src/components/ChatInput/ChatInput.tsx`:
- Around line 89-100: The typing timeout (typingTimeoutRef) in ChatInput is
never cleared on unmount; add a useEffect cleanup in the ChatInput component
that clears typingTimeoutRef.current (clearTimeout) and calls
onTypingChange?.(false) to ensure the timeout is cancelled and the typing
indicator is reset when the component unmounts. Locate the logic using
typingTimeoutRef and onTypingChange in ChatInput.tsx and implement the cleanup
inside a return () => { ... } of a useEffect so no stale timeout/state updates
remain after unmount.
In `@packages/ai-chat/src/stream/client.ts`:
- Around line 202-212: The disconnect() method is silently swallowing errors
from this._removePresence(); replace the .catch(() => {}) with a catch that logs
the error and context (e.g., "Error removing presence for user" and the user
id/this.options.user) using the instance logger (this.logger.error) or
console.error if no logger exists, so failures in _removePresence() are recorded
while still allowing disconnect to proceed; ensure you reference disconnect(),
_removePresence(), this.options.user and this._isConnected when adding the log.
In `@packages/ai-chat/src/stream/materialize.ts`:
- Around line 72-79: The sort can be unstable when createdAt parses to NaN;
update the comparator used to create sorted (over chunks) to defensively parse
createdAt into a finite timestamp (e.g., parse Date(String(createdAt)).getTime()
then if not finite fallback to 0) before comparing, and keep the existing
fallback for _seq (a._seq and b._seq) intact; ensure you use Number.isFinite or
isNaN checks so malformed dates are treated as 0 and the tie-breaker _seq
ordering remains deterministic.
In `@packages/db/drizzle/meta/_journal.json`:
- Around line 99-100: The migration journal entry for "0013_add_chat_tables" in
the _journal.json was edited manually; revert that file to the committed state
and do not hand-edit migration journal or snapshot files (e.g., _journal.json
and any .sql snapshots). Instead regenerate the migration correctly using the
project's Drizzle migration workflow/tooling (run the official
migration/generate command you use in CI/local dev) so the journal updates are
produced automatically and commit the regenerated artifacts.
🧹 Nitpick comments (20)
apps/mobile/screens/(authenticated)/chat/[sessionId]/components/ChatMessage/ChatMessage.tsx (2)
20-24: Clarify the role header display condition.The condition
!isCurrentUser && message.role !== "user"appears redundant. IfisCurrentUseris derived frommessage.role === "user", this check would never show the role header. IfisCurrentUseris determined differently (e.g., by sender ID), then the logic makes sense but the relationship should be clarified.Consider simplifying or adding a comment explaining when these values could diverge:
- {!isCurrentUser && message.role !== "user" && ( + {/* Show role label for non-user messages (e.g., assistant, system) */} + {message.role !== "user" && (
34-36: Consider defensive date handling.
formatTimeassumesdateis a validDateobject. Ifmessage.createdAtcould be a string or invalid,toLocaleTimeStringwould return "Invalid Date".🛡️ Optional defensive handling
function formatTime(date: Date): string { + if (!(date instanceof Date) || isNaN(date.getTime())) { + return ""; + } return date.toLocaleTimeString([], { hour: "2-digit", minute: "2-digit" }); }apps/mobile/screens/(authenticated)/chat/[sessionId]/components/ChatInput/ChatInput.tsx (2)
44-44: Extract hardcoded colors to theme tokens.The placeholder color
#9ca3afis hardcoded, which breaks theme consistency. Consider using a theme token or referencing the design system's muted foreground color.
59-63: Hardcoded icon colors may break theming.The Send icon uses hardcoded
#ffffffand#9ca3af. These should ideally come from theme tokens to support dark/light mode properly.packages/ai-chat/src/components/ChatInput/ChatInput.tsx (2)
85-85: Extract magic numbers to named constants.The values
200(max textarea height) and2000(typing debounce ms) appear as magic numbers. Per coding guidelines, these should be extracted to named constants at module top for clarity.♻️ Suggested extraction
+const MAX_TEXTAREA_HEIGHT_PX = 200; +const TYPING_DEBOUNCE_MS = 2000; + export function ChatInput({Then reference
MAX_TEXTAREA_HEIGHT_PXon lines 85 and 115, andTYPING_DEBOUNCE_MSon line 97.Also applies to: 97-97, 115-115
45-47:setValueis recreated on every render in controlled mode.When
isControlledis true,setValueis a new arrow function on each render, causinghandleSubmitandhandleChangeto be recreated despiteuseCallback. Consider memoizingsetValue:♻️ Optional memoization
- const setValue = isControlled - ? (v: string) => controlledOnChange?.(v) - : setInternalValue; + const setValue = useCallback( + (v: string) => { + if (isControlled) { + controlledOnChange?.(v); + } else { + setInternalValue(v); + } + }, + [isControlled, controlledOnChange], + );packages/ai-chat/package.json (1)
25-25: React should be a peer dependency only, not a direct dependency.Having
reactas both a direct dependency (pinned to19.1.0) and a peer dependency (^18.0.0 || ^19.0.0) is problematic for a shared package. This can cause multiple React instances in the bundle, leading to hooks errors and increased bundle size.Remove
reactfromdependenciesand keep it only inpeerDependencies.♻️ Proposed fix
"dependencies": { "@anthropic-ai/claude-agent-sdk": "^0.2.19", "@anthropic-ai/sdk": "^0.71.2", "@durable-streams/client": "^0.2.0", "@durable-streams/state": "^0.2.0", "@superset/ui": "workspace:*", "@tanstack/db": "^0.5.24", "@tanstack/react-db": "^0.1.68", "lucide-react": "^0.560.0", - "react": "19.1.0", "zod": "^4.3.5" },Also applies to: 34-36
apps/mobile/screens/(authenticated)/chat/[sessionId]/ChatScreen.tsx (3)
92-98: Extract magic number and fix useEffect dependencies.Two issues here:
- The
100ms delay is a magic number—extract to a named constant.- Including both
streamingMessage?.contentandstreamingMessagein dependencies is redundant; the object reference change already triggers the effect.♻️ Proposed fix
Add constant at module top:
const SCROLL_DELAY_MS = 100;Then update the effect:
useEffect(() => { if (messages.length > 0 || streamingMessage) { setTimeout(() => { flatListRef.current?.scrollToEnd({ animated: true }); - }, 100); + }, SCROLL_DELAY_MS); } - }, [messages.length, streamingMessage?.content, streamingMessage]); + }, [messages.length, streamingMessage]);As per coding guidelines: avoid magic numbers by extracting them to named constants at module top.
100-120: Remove unnecessarystreamsUrlfrom dependency array.The static analysis correctly identifies that
streamsUrlis derived fromenv.EXPO_PUBLIC_STREAMS_URL(line 73), which is a module-level constant that won't change between renders. Including it in the dependency array is unnecessary and can cause confusion.♻️ Proposed fix
const handleSend = useCallback( async (content: string) => { if (!user?.userId || !streamsUrl || !sessionId) { console.error( "[chat] Cannot send message: missing user, URL, or sessionId", ); return; } try { await sendMessageToStream({ proxyUrl: streamsUrl, sessionId, userId: user.userId, content, }); } catch (err) { console.error("[chat] Failed to send message:", err); } }, - [user?.userId, streamsUrl, sessionId], + [user?.userId, sessionId], );
167-167: Redundant identical values forkeyboardVerticalOffset.Both branches of the ternary evaluate to
0, making the conditional unnecessary.♻️ Proposed fix
<KeyboardAvoidingView className="flex-1 bg-background" behavior={Platform.OS === "ios" ? "padding" : "height"} - keyboardVerticalOffset={Platform.OS === "ios" ? 0 : 0} + keyboardVerticalOffset={0} >packages/ai-chat/src/stream/actions.ts (1)
113-127: Redundantvalueobject sent with delete operation.When
contentis empty (line 124), the operation is"delete"but a fullvalueobject is still constructed and sent. For delete operations, thevalueis typically unnecessary and increases payload size.♻️ Suggested optimization
updateDraft: async (content: string) => { + const isDelete = !content; await appendToStream(streamUrl, [ { type: "draft", key: user.userId, - value: { + ...(isDelete ? {} : { value: { userId: user.userId, userName: user.name, content, updatedAt: new Date().toISOString(), - }, - headers: { operation: content ? "upsert" : "delete" }, + }}), + headers: { operation: isDelete ? "delete" : "upsert" }, }, ]); },apps/streams/src/session-registry.ts (1)
27-32:list()re-sorts on every call.Sorting happens on each
list()invocation. If this is called frequently, consider caching the sorted array and invalidating onregister().packages/ai-chat/src/stream/client.ts (2)
124-127: Type assertion throughunknownbypasses type safety.The double cast
as unknown as SessionCollectionsdisables type checking entirely. If_db.collectionsstructure changes or differs from expectations, TypeScript won't catch the mismatch. Consider adding a runtime shape check in development builds or documenting why this cast is safe.
349-362: Network call without timeout or abort signal.The
fetchcall in_appendToStreamhas no timeout or abort signal, which could cause it to hang indefinitely on network issues. Consider passingthis._abortController.signalor a separate timeout signal for resilience.♻️ Proposed fix to add abort signal
private async _appendToStream(events: unknown[]): Promise<void> { const response = await fetch( `${this.options.proxyUrl}/streams/${this.sessionId}`, { method: "POST", headers: { "Content-Type": "application/json" }, body: JSON.stringify(events), + signal: this._abortController.signal, }, );packages/ai-chat/src/stream/useChatSession.ts (1)
181-192: Unnecessary memoization and extraneous dependency.Line 182: The
useMemowrappingdraftRowsis a no-op—it returns the input unchanged.Lines 185-192: The static analysis correctly flags that
clientOptions.useras a dependency causes re-computation when the user object reference changes, even ifuserIdhasn't. Consider extractinguser?.userIdto a stable reference.♻️ Proposed fix
- // All drafts - const drafts = useMemo((): StreamDraft[] => draftRows, [draftRows]); + // All drafts (no memo needed - draftRows is already stable from useCollectionData) + const drafts = draftRows; + const currentUserId = clientOptions.user?.userId; + // Current user's draft const draft = useMemo((): string => { - const user = clientOptions.user; - if (!user) return ""; - const myDraft = draftRows.find( - (d: StreamDraft) => d.userId === user.userId, - ); + if (!currentUserId) return ""; + const myDraft = draftRows.find((d: StreamDraft) => d.userId === currentUserId); return myDraft?.content ?? ""; - }, [draftRows, clientOptions.user]); + }, [draftRows, currentUserId]);packages/ai-chat/src/stream/materialize.ts (1)
281-286: Silent catch for partial JSON is intentional but could benefit from debug logging.The empty catch block handles expected partial JSON during streaming. While the design is correct (streaming JSON is often incomplete), consider adding optional debug-level logging for troubleshooting stream reconstruction issues.
docs/ai-chat-plan.md (1)
7-35: Add language specifier to fenced code blocks.The architecture diagram and directory structure blocks (lines 7, 281, 381, 404) lack language specifiers. Use
textorplaintextfor ASCII diagrams and directory trees to satisfy markdown lint.📝 Example fix for line 7
-``` +```text Any Client (Web/Mobile) API Desktop (Electron)apps/desktop/src/renderer/screens/main/components/WorkspaceSidebar/WorkspaceSidebarHeader/WorkspaceSidebarHeader.tsx (1)
91-107: Extract tooltip delay into a named constant.
This avoids repeating the same literal and clarifies intent.♻️ Suggested refactor
+const TOOLTIP_DELAY_MS = 300; ... - <Tooltip delayDuration={300}> + <Tooltip delayDuration={TOOLTIP_DELAY_MS}> ... - <Tooltip delayDuration={300}> + <Tooltip delayDuration={TOOLTIP_DELAY_MS}> ... - <Tooltip delayDuration={300}> + <Tooltip delayDuration={TOOLTIP_DELAY_MS}>As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
apps/desktop/src/renderer/routes/_authenticated/_dashboard/chats/stores/chatStore.ts (2)
29-42: Add an operation suffix to the sync log prefix.This keeps logs consistent and easier to filter.
As per coding guidelines, Use prefixed console logging with pattern [domain/operation] message for all logging.♻️ Suggested tweak
- console.warn("[chatStore] Failed to sync session to streams server:", err); + console.warn( + "[chatStore/syncToStreamsServer] Failed to sync session to streams server:", + err, + );
19-24: Use a params object forrenameSession.This avoids positional ambiguity and aligns with the codebase convention. Please update call sites accordingly.
As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.♻️ Suggested refactor
interface ChatStore { sessions: ChatSession[]; createSession: (name?: string) => ChatSession; deleteSession: (id: string) => void; - renameSession: (id: string, name: string) => void; + renameSession: (params: { id: string; name: string }) => void; } @@ - renameSession: (id: string, name: string) => { + renameSession: ({ id, name }) => { set((state) => ({ sessions: state.sessions.map((s) => s.id === idAlso applies to: 76-83
| NEXT_PUBLIC_STREAMS_URL=http://localhost:8080 | ||
| EXPO_PUBLIC_STREAMS_URL=http://localhost:8080 |
There was a problem hiding this comment.
Reorder keys to satisfy dotenv-linter.
The linter expects EXPO_PUBLIC_STREAMS_URL before NEXT_PUBLIC_STREAMS_URL. Reordering will clear the warning.
🔧 Suggested reorder
-NEXT_PUBLIC_STREAMS_URL=http://localhost:8080
-EXPO_PUBLIC_STREAMS_URL=http://localhost:8080
+EXPO_PUBLIC_STREAMS_URL=http://localhost:8080
+NEXT_PUBLIC_STREAMS_URL=http://localhost:8080📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| NEXT_PUBLIC_STREAMS_URL=http://localhost:8080 | |
| EXPO_PUBLIC_STREAMS_URL=http://localhost:8080 | |
| EXPO_PUBLIC_STREAMS_URL=http://localhost:8080 | |
| NEXT_PUBLIC_STREAMS_URL=http://localhost:8080 |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 34-34: [UnorderedKey] The EXPO_PUBLIC_STREAMS_URL key should go before the NEXT_PUBLIC_STREAMS_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.example around lines 33 - 34, Reorder the two environment variable
entries so EXPO_PUBLIC_STREAMS_URL appears before NEXT_PUBLIC_STREAMS_URL in the
.env.example file to satisfy dotenv-linter; specifically swap the lines
containing EXPO_PUBLIC_STREAMS_URL and NEXT_PUBLIC_STREAMS_URL so the key
EXPO_PUBLIC_STREAMS_URL comes first.
| // Claude Code binary - bundled for AI chat functionality | ||
| { | ||
| from: "resources/bin/${platform}-${arch}", | ||
| to: "bin", | ||
| filter: ["**/*"], | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find apps/desktop -type f -name "*download*" | head -20Repository: superset-sh/superset
Length of output: 110
🏁 Script executed:
cat -n apps/desktop/scripts/download-claude-binary.ts 2>/dev/null || echo "File not found at expected path"Repository: superset-sh/superset
Length of output: 12524
🌐 Web query:
electron-builder extraResources macro expansion platform arch variables
💡 Result:
In electron-builder, extraResources supports the same file-macro expansion as files patterns, and macros can be used in:
- glob patterns you put in
extraResources FileSet.fromandFileSet.towhen you use the object form ofextraResources([{ from, to, filter }]) [1]
Supported macros relevant to platform/arch:
${os}→mac,linux, orwin(target platform) [2]${arch}→ e.g.ia32,x64(and commonlyarm64in practice) [2]
Also available (often useful alongside): ${name}, ${productName}, ${version}, and ${env.MY_VAR} [2].
Example: platform + arch–specific resources
{
"build": {
"extraResources": [
{
"from": "resources/${os}/${arch}",
"to": "resources/native",
"filter": ["**/*"]
}
]
}
}This copies resources/mac/x64/** (or resources/win/ia32/**, etc.) into the app’s packaged resources/native directory [1][2].
Notes:
- If an
archisn’t being set for a given build,${arch}is removed (with some cleanup of adjacent separators) rather than left literal [2]. - Use
${os}(documented);${platform}is not part of the documented macro set in electron-builder’s file macros list [2].
Sources: electron-builder “Application Contents” (extraResources + FileSet) [1], electron-builder “File Macros” list and behavior [2].
🌐 Web query:
electron-builder ${platform} ${arch} variable substitution path
💡 Result:
electron-builder supports “file macros” (variable substitution) in several path-like config fields, including output directories and FileSet from/to, not just artifactName.
Macros you’re asking about
${platform}→ Nodeprocess.platform(darwin,win32,linux) (stackoverflow.com)${arch}→ build arch (x64,ia32,arm64, etc.) (stackoverflow.com)
(Also common:${os}→mac/win/linux.) (stackoverflow.com)
Put platform/arch into the output path
{
"build": {
"directories": {
"output": "releases/${platform}/${arch}"
}
}
}This creates per-platform/per-arch folders under releases/. (stackoverflow.com)
Use platform/arch in copy paths (extraResources / extraFiles)
electron-builder allows file macros in FileSet from and to paths: (electron.build)
{
"build": {
"extraResources": [
{
"from": "bin/${platform}/${arch}/mytool",
"to": "bin/mytool"
}
]
}
}Use platform/arch in the artifact filename
{
"build": {
"artifactName": "${productName}-${version}-${platform}-${arch}.${ext}"
}
}artifactName explicitly supports these macros. (electron.build)
Citations:
- 1: https://stackoverflow.com/questions/61167439/electron-nsis-web-how-to-change-artifacts-names?utm_source=openai
- 2: https://stackoverflow.com/questions/61167439/electron-nsis-web-how-to-change-artifacts-names?utm_source=openai
- 3: https://stackoverflow.com/questions/61167439/electron-nsis-web-how-to-change-artifacts-names?utm_source=openai
- 4: https://stackoverflow.com/questions/62660396/os-specific-directories-for-binary-generated-by-electron-builder?utm_source=openai
- 5: https://www.electron.build/contents.html?utm_source=openai
- 6: https://www.electron.build/configuration.html?utm_source=openai
${platform} is not a valid electron-builder macro; use ${os} instead, but platform names still won't match.
The extraResources.from path uses ${platform} and ${arch}, but electron-builder only expands ${os} (which yields mac/linux/win) and ${arch}. More critically, the download script creates directories using Node's naming (darwin-arm64, linux-x64, win32-x64), while electron-builder would resolve to mac-x64, linux-x64, win-x64. The paths won't align, and the binary won't be bundled.
Update the from path to match the directory structure created by the download script. Either:
- Update the download script to use electron-builder's platform naming convention, or
- Update
electron-builder.tsto reference the actual directory structure created (e.g., conditional configuration per platform, or refactor how binaries are organized).
🤖 Prompt for AI Agents
In `@apps/desktop/electron-builder.ts` around lines 59 - 64, The
extraResources.from currently uses "resources/bin/${platform}-${arch}" which is
invalid for electron-builder and mismatches the download script's Node-style
dirs (e.g., darwin-arm64, linux-x64, win32-x64); update the config referenced by
extraResources.from to point to the actual directory names created by the
download script (or change the download script to emit electron-builder names).
Concretely, replace the single-template from with platform-aware logic: map
electron-builder ${os}/${arch} to the download-script names (or use
process.platform mapping such as darwin -> darwin, win32 -> win32, linux ->
linux and include arch) so the path used by extraResources.from matches the real
folders (or alternatively change the downloader to write mac/linux/win folders
to match electron-builder). Ensure the path string used by extraResources.from
exactly matches the download script's output naming convention.
| if (enableDurableStream) { | ||
| try { | ||
| await createProducer(sessionId); | ||
| console.log(`[claude/session] Durable stream created for ${sessionId}`); | ||
| } catch (error) { | ||
| console.error(`[claude/session] Failed to create stream:`, error); | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Read the session-manager file around the specified lines
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts | sed -n '200,240p'Repository: superset-sh/superset
Length of output: 1393
🏁 Script executed:
# Get full context of startSession function
ast-grep --pattern 'startSession($_, $_) {
$$$
}' apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for startSession callers
rg -n "startSession\(" apps/desktop/src/lib/trpc -g '*.ts' -g '*.tsx' -A 3Repository: superset-sh/superset
Length of output: 836
🏁 Script executed:
# Read the caller context in index.ts
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/index.ts | sed -n '20,50p'Repository: superset-sh/superset
Length of output: 1016
🏁 Script executed:
# Check if there are other return paths in startSession
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts | sed -n '199,280p'Repository: superset-sh/superset
Length of output: 2649
🏁 Script executed:
# Check what happens after startSession is called in index.ts
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/index.ts | sed -n '25,80p'Repository: superset-sh/superset
Length of output: 1931
🏁 Script executed:
# Check where sessionProducers is populated (should only be createProducer)
rg -n "sessionProducers\\.set\\|sessionProducers\\[" apps/desktop/src/lib/trpc -g '*.ts' -g '*.tsx'Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check createProducer to understand what it does
rg -n "createProducer" apps/desktop/src/lib/trpc -g '*.ts' -g '*.tsx' -B 2 -A 2Repository: superset-sh/superset
Length of output: 1324
🏁 Script executed:
# Read createProducer implementation to understand what it does
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts | sed -n '68,100p'Repository: superset-sh/superset
Length of output: 1243
🏁 Script executed:
# Check if createProducer populates sessionProducers (likely in the function)
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts | sed -n '70,120p'Repository: superset-sh/superset
Length of output: 2069
Throw TRPCError if durable stream initialization fails, rather than silently creating a non-functional session.
If createProducer fails, the session is created without a message watcher and cannot process user messages. Currently the error is swallowed and the tRPC mutation returns { success: true }, misleading the client.
🔧 Suggested fix
if (enableDurableStream) {
try {
await createProducer(sessionId);
console.log(`[claude/session] Durable stream created for ${sessionId}`);
} catch (error) {
- console.error(`[claude/session] Failed to create stream:`, error);
+ console.error(`[claude/session] Failed to create stream:`, error);
+ throw new TRPCError({
+ code: "INTERNAL_SERVER_ERROR",
+ message: "Failed to initialize durable stream for session",
+ cause: error,
+ });
}
}🤖 Prompt for AI Agents
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`
around lines 217 - 224, If durable stream initialization fails in the session
creation path, do not swallow the error; instead throw a TRPCError so the
mutation fails and the client is not misled. In the block where
enableDurableStream is checked and createProducer(sessionId) is awaited, catch
the error and rethrow a TRPCError (e.g., new TRPCError({ code: 'INTERNAL',
message: 'Failed to initialize durable stream', cause: error })) so the session
creation aborts; reference createProducer, enableDurableStream, and sessionId in
your change and import TRPCError from `@trpc/server` if not already imported.
| function readBody(req: import("node:http").IncomingMessage): Promise<string> { | ||
| return new Promise((resolve, reject) => { | ||
| const chunks: Buffer[] = []; | ||
| req.on("data", (chunk) => chunks.push(chunk)); | ||
| req.on("end", () => resolve(Buffer.concat(chunks).toString("utf-8"))); | ||
| req.on("error", reject); | ||
| }); | ||
| } |
There was a problem hiding this comment.
No request body size limit in readBody.
The readBody function accumulates all chunks without enforcing a maximum size. A malicious client could send an arbitrarily large body to exhaust server memory.
🛡️ Suggested fix with size limit
+const MAX_BODY_SIZE = 1024 * 1024; // 1MB limit
function readBody(req: import("node:http").IncomingMessage): Promise<string> {
return new Promise((resolve, reject) => {
const chunks: Buffer[] = [];
+ let totalSize = 0;
- req.on("data", (chunk) => chunks.push(chunk));
+ req.on("data", (chunk) => {
+ totalSize += chunk.length;
+ if (totalSize > MAX_BODY_SIZE) {
+ req.destroy();
+ reject(new Error("Request body too large"));
+ return;
+ }
+ chunks.push(chunk);
+ });
req.on("end", () => resolve(Buffer.concat(chunks).toString("utf-8")));
req.on("error", reject);
});
}🤖 Prompt for AI Agents
In `@apps/streams/src/index.ts` around lines 125 - 132, The readBody function
currently accumulates request chunks without a size limit, allowing unbounded
memory use; update readBody(req: IncomingMessage) to enforce a configurable
maxBytes (e.g., 1MB) by tracking totalLength as chunks arrive, and if
totalLength exceeds maxBytes immediately remove listeners, call reject with a
clear error (e.g., PayloadTooLargeError or a custom Error) and destroy the
request stream; ensure the normal "end" and "error" handlers still
resolve/reject correctly and that listeners are cleaned up on all paths so no
memory leaks remain.
| private load(): void { | ||
| try { | ||
| if (existsSync(this.filePath)) { | ||
| const data = readFileSync(this.filePath, "utf-8"); | ||
| const sessions: SessionInfo[] = JSON.parse(data); | ||
| for (const session of sessions) { | ||
| this.sessions.set(session.sessionId, session); | ||
| } | ||
| console.log( | ||
| `[session-registry] Loaded ${sessions.length} sessions from disk`, | ||
| ); | ||
| } | ||
| } catch (error) { | ||
| console.error("[session-registry] Failed to load sessions:", error); | ||
| } | ||
| } |
There was a problem hiding this comment.
Parsed JSON not validated against SessionInfo schema.
The load() method parses JSON and directly casts to SessionInfo[] without validation. If the file is corrupted or has an unexpected structure, this could cause runtime errors elsewhere.
🛡️ Suggested fix with validation
private load(): void {
try {
if (existsSync(this.filePath)) {
const data = readFileSync(this.filePath, "utf-8");
- const sessions: SessionInfo[] = JSON.parse(data);
+ const parsed = JSON.parse(data);
+ if (!Array.isArray(parsed)) {
+ console.error("[session-registry] Invalid sessions file format");
+ return;
+ }
for (const session of sessions) {
+ if (!session.sessionId || !session.title || !session.createdAt) {
+ console.warn("[session-registry] Skipping invalid session entry");
+ continue;
+ }
this.sessions.set(session.sessionId, session);
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private load(): void { | |
| try { | |
| if (existsSync(this.filePath)) { | |
| const data = readFileSync(this.filePath, "utf-8"); | |
| const sessions: SessionInfo[] = JSON.parse(data); | |
| for (const session of sessions) { | |
| this.sessions.set(session.sessionId, session); | |
| } | |
| console.log( | |
| `[session-registry] Loaded ${sessions.length} sessions from disk`, | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("[session-registry] Failed to load sessions:", error); | |
| } | |
| } | |
| private load(): void { | |
| try { | |
| if (existsSync(this.filePath)) { | |
| const data = readFileSync(this.filePath, "utf-8"); | |
| const parsed = JSON.parse(data); | |
| if (!Array.isArray(parsed)) { | |
| console.error("[session-registry] Invalid sessions file format"); | |
| return; | |
| } | |
| for (const session of parsed) { | |
| if (!session.sessionId || !session.title || !session.createdAt) { | |
| console.warn("[session-registry] Skipping invalid session entry"); | |
| continue; | |
| } | |
| this.sessions.set(session.sessionId, session); | |
| } | |
| console.log( | |
| `[session-registry] Loaded ${parsed.length} sessions from disk`, | |
| ); | |
| } | |
| } catch (error) { | |
| console.error("[session-registry] Failed to load sessions:", error); | |
| } | |
| } |
🤖 Prompt for AI Agents
In `@apps/streams/src/session-registry.ts` around lines 68 - 83, The load() method
currently parses file JSON and assumes it matches SessionInfo[]; add validation
to avoid corrupt data propagation by validating the parsed value before
inserting into this.sessions: in load() after JSON.parse(data) validate that the
top-level value is an array and each element conforms to the SessionInfo shape
(required fields like sessionId, userId, createdAt, etc.) using a schema
validator or a small type-guard function, skip or filter out invalid entries,
log warnings for any malformed items, and only call
this.sessions.set(session.sessionId, session) for validated SessionInfo objects
so downstream code using SessionInfo is safe.
| if (typingTimeoutRef.current) { | ||
| clearTimeout(typingTimeoutRef.current); | ||
| } | ||
|
|
||
| if (newValue.trim()) { | ||
| onTypingChange?.(true); | ||
| typingTimeoutRef.current = setTimeout(() => { | ||
| onTypingChange?.(false); | ||
| }, 2000); | ||
| } else { | ||
| onTypingChange?.(false); | ||
| } |
There was a problem hiding this comment.
Missing cleanup for typing timeout on unmount.
The typing indicator timeout (typingTimeoutRef) is not cleared when the component unmounts. This could cause a state update on an unmounted component or leave a stale typing indicator.
🐛 Proposed fix
Add a cleanup effect:
+import { type KeyboardEvent, useCallback, useEffect, useRef, useState } from "react";
// ... inside ChatInput component, after typingTimeoutRef declaration:
+ useEffect(() => {
+ return () => {
+ if (typingTimeoutRef.current) {
+ clearTimeout(typingTimeoutRef.current);
+ }
+ };
+ }, []);🤖 Prompt for AI Agents
In `@packages/ai-chat/src/components/ChatInput/ChatInput.tsx` around lines 89 -
100, The typing timeout (typingTimeoutRef) in ChatInput is never cleared on
unmount; add a useEffect cleanup in the ChatInput component that clears
typingTimeoutRef.current (clearTimeout) and calls onTypingChange?.(false) to
ensure the timeout is cancelled and the typing indicator is reset when the
component unmounts. Locate the logic using typingTimeoutRef and onTypingChange
in ChatInput.tsx and implement the cleanup inside a return () => { ... } of a
useEffect so no stale timeout/state updates remain after unmount.
| disconnect(): void { | ||
| // Remove presence before disconnecting | ||
| if (this.options.user && this._isConnected) { | ||
| this._removePresence().catch(() => {}); | ||
| } | ||
|
|
||
| this._db.close(); | ||
| this._abortController.abort(); | ||
| this._isConnected = false; | ||
| this._connectionStatus = "disconnected"; | ||
| } |
There was a problem hiding this comment.
Silent error swallowing in disconnect().
Line 205 uses .catch(() => {}) which silently discards errors from _removePresence(). Per coding guidelines, errors should at minimum be logged with context.
🛠️ Proposed fix
disconnect(): void {
// Remove presence before disconnecting
if (this.options.user && this._isConnected) {
- this._removePresence().catch(() => {});
+ this._removePresence().catch((err) => {
+ console.error("[ai-chat/disconnect] Failed to remove presence:", err);
+ });
}
this._db.close();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| disconnect(): void { | |
| // Remove presence before disconnecting | |
| if (this.options.user && this._isConnected) { | |
| this._removePresence().catch(() => {}); | |
| } | |
| this._db.close(); | |
| this._abortController.abort(); | |
| this._isConnected = false; | |
| this._connectionStatus = "disconnected"; | |
| } | |
| disconnect(): void { | |
| // Remove presence before disconnecting | |
| if (this.options.user && this._isConnected) { | |
| this._removePresence().catch((err) => { | |
| console.error("[ai-chat/disconnect] Failed to remove presence:", err); | |
| }); | |
| } | |
| this._db.close(); | |
| this._abortController.abort(); | |
| this._isConnected = false; | |
| this._connectionStatus = "disconnected"; | |
| } |
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/client.ts` around lines 202 - 212, The
disconnect() method is silently swallowing errors from this._removePresence();
replace the .catch(() => {}) with a catch that logs the error and context (e.g.,
"Error removing presence for user" and the user id/this.options.user) using the
instance logger (this.logger.error) or console.error if no logger exists, so
failures in _removePresence() are recorded while still allowing disconnect to
proceed; ensure you reference disconnect(), _removePresence(), this.options.user
and this._isConnected when adding the log.
| const sorted = [...chunks].sort((a, b) => { | ||
| const aTime = a.createdAt ? new Date(String(a.createdAt)).getTime() : 0; | ||
| const bTime = b.createdAt ? new Date(String(b.createdAt)).getTime() : 0; | ||
| if (aTime !== bTime) return aTime - bTime; | ||
| const aSeq = typeof a._seq === "number" ? a._seq : 0; | ||
| const bSeq = typeof b._seq === "number" ? b._seq : 0; | ||
| return aSeq - bSeq; | ||
| }); |
There was a problem hiding this comment.
Sort may be unstable with malformed dates.
If createdAt is present but malformed, new Date(String(...)).getTime() returns NaN. Since NaN comparisons always return false, this could cause unstable sort ordering. Consider using a helper that validates the parsed time.
🛠️ Proposed fix
+const parseTime = (value: unknown): number => {
+ if (!value) return 0;
+ const time = new Date(String(value)).getTime();
+ return Number.isNaN(time) ? 0 : time;
+};
+
const sorted = [...chunks].sort((a, b) => {
- const aTime = a.createdAt ? new Date(String(a.createdAt)).getTime() : 0;
- const bTime = b.createdAt ? new Date(String(b.createdAt)).getTime() : 0;
+ const aTime = parseTime(a.createdAt);
+ const bTime = parseTime(b.createdAt);
if (aTime !== bTime) return aTime - bTime;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const sorted = [...chunks].sort((a, b) => { | |
| const aTime = a.createdAt ? new Date(String(a.createdAt)).getTime() : 0; | |
| const bTime = b.createdAt ? new Date(String(b.createdAt)).getTime() : 0; | |
| if (aTime !== bTime) return aTime - bTime; | |
| const aSeq = typeof a._seq === "number" ? a._seq : 0; | |
| const bSeq = typeof b._seq === "number" ? b._seq : 0; | |
| return aSeq - bSeq; | |
| }); | |
| const parseTime = (value: unknown): number => { | |
| if (!value) return 0; | |
| const time = new Date(String(value)).getTime(); | |
| return Number.isNaN(time) ? 0 : time; | |
| }; | |
| const sorted = [...chunks].sort((a, b) => { | |
| const aTime = parseTime(a.createdAt); | |
| const bTime = parseTime(b.createdAt); | |
| if (aTime !== bTime) return aTime - bTime; | |
| const aSeq = typeof a._seq === "number" ? a._seq : 0; | |
| const bSeq = typeof b._seq === "number" ? b._seq : 0; | |
| return aSeq - bSeq; | |
| }); |
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/materialize.ts` around lines 72 - 79, The sort
can be unstable when createdAt parses to NaN; update the comparator used to
create sorted (over chunks) to defensively parse createdAt into a finite
timestamp (e.g., parse Date(String(createdAt)).getTime() then if not finite
fallback to 0) before comparing, and keep the existing fallback for _seq (a._seq
and b._seq) intact; ensure you use Number.isFinite or isNaN checks so malformed
dates are treated as 0 and the tie-breaker _seq ordering remains deterministic.
| "when": 1769296997267, | ||
| "tag": "0013_add_chat_tables", |
There was a problem hiding this comment.
Do not manually edit drizzle migration journal entries.
These edits are in packages/db/drizzle/meta/_journal.json, which must not be changed manually. Please revert and regenerate via the official migration workflow/tooling instead.
As per coding guidelines: Never manually edit migration files in packages/db/drizzle/ including .sql files, meta/_journal.json, and snapshot files.
🤖 Prompt for AI Agents
In `@packages/db/drizzle/meta/_journal.json` around lines 99 - 100, The migration
journal entry for "0013_add_chat_tables" in the _journal.json was edited
manually; revert that file to the committed state and do not hand-edit migration
journal or snapshot files (e.g., _journal.json and any .sql snapshots). Instead
regenerate the migration correctly using the project's Drizzle migration
workflow/tooling (run the official migration/generate command you use in
CI/local dev) so the journal updates are produced automatically and commit the
regenerated artifacts.
There was a problem hiding this comment.
Actionable comments posted: 8
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/desktop/src/lib/trpc/routers/ui-state/index.ts (1)
33-47:⚠️ Potential issue | 🟠 MajorRequire chat.sessionId when type is "chat".
The current schema allows
type: "chat"panes without chat state, which can persist invalid UI state and cause runtime errors. Use a discriminated union to enforce that chat panes must include the sessionId.🛡️ Example with a discriminated union
-const paneSchema = z.object({ - id: z.string(), - tabId: z.string(), - type: z.enum(["terminal", "webview", "file-viewer", "chat"]), - name: z.string(), - isNew: z.boolean().optional(), - status: z.enum(["idle", "working", "permission", "review"]).optional(), - initialCommands: z.array(z.string()).optional(), - initialCwd: z.string().optional(), - url: z.string().optional(), - cwd: z.string().nullable().optional(), - cwdConfirmed: z.boolean().optional(), - fileViewer: fileViewerStateSchema.optional(), - chat: z.object({ sessionId: z.string() }).optional(), -}); +const basePaneSchema = z.object({ + id: z.string(), + tabId: z.string(), + name: z.string(), + isNew: z.boolean().optional(), + status: z.enum(["idle", "working", "permission", "review"]).optional(), + initialCommands: z.array(z.string()).optional(), + initialCwd: z.string().optional(), + url: z.string().optional(), + cwd: z.string().nullable().optional(), + cwdConfirmed: z.boolean().optional(), + fileViewer: fileViewerStateSchema.optional(), +}); + +const chatPaneSchema = basePaneSchema.extend({ + type: z.literal("chat"), + chat: z.object({ sessionId: z.string() }), +}); +const nonChatPaneSchema = basePaneSchema.extend({ + type: z.enum(["terminal", "webview", "file-viewer"]), + chat: z.never().optional(), +}); + +const paneSchema = z.discriminatedUnion("type", [ + chatPaneSchema, + nonChatPaneSchema, +]);
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`:
- Around line 76-82: The catch block that falls back from DurableStream.create
to DurableStream.connect is silently swallowing the error; update the catch to
log the caught error with context (including streamOpts) before attempting
DurableStream.connect so failures are visible; locate the try/catch around
DurableStream.create in session-manager.ts and add a processLogger (or the
existing logger) call that includes a descriptive message, the error object, and
relevant identifiers prior to calling DurableStream.connect(streamOpts).
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/chats/components/ChatMessageList/ChatMessageList.tsx`:
- Around line 50-59: The Streaming message row uses a constant key ("streaming")
inside the messages.map rendering, which can cause React to reuse components
incorrectly; update the key prop on the ChatMessage returned in the messages.map
branch (where "type" in msg && msg.type === "streaming") to be unique per item —
for example combine a unique identifier from the message (msg.id or similar) or
fallback to the map index (e.g., `streaming-${index}`) so each streaming row has
a distinct key.
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ChatInputButton/ChatInputButton.tsx`:
- Around line 15-18: The click handler handleClick should catch failures from
createStream so navigation doesn't silently abort: wrap the await
createStream(STREAM_SERVER_URL, session.id) call in a try/catch inside the
useCallback for handleClick, log the error with a contextual prefix like
"[chat/createStream]" plus the error details, and then decide whether to proceed
with navigate({ to: "/chats/$chatId", params: { chatId: session.id } }) or
surface an error state (e.g., set an error flag or show a toast) instead of
swallowing the exception; keep references to createSession, createStream and
navigate when implementing the change.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`:
- Around line 101-109: handleAddChat can leave an orphaned session if
createStream throws; wrap the createStream + addChatPane sequence in a
try/catch, and on error log with the prefixed pattern (e.g.
console.error(`[streams/create] Failed to create stream for session
${session.id}:`, err)) and perform rollback (remove the persisted session using
your session deletion API/function—e.g. removeSession(session.id) or
deleteSession(session.id)) so the store stays consistent, and surface user
feedback or rethrow as appropriate; reference symbols: handleAddChat,
createSession, createStream, addChatPane, STREAM_SERVER_URL, and the session.id
returned by createSession.
In `@apps/mobile/screens/`(authenticated)/chat/[sessionId]/ChatScreen.tsx:
- Around line 100-120: The handleSend callback is missing streamsUrl (and the
sendMessageToStream reference if not stable) in its dependency array and uses
the wrong log prefix; update the dependency array for handleSend to include
streamsUrl (and sendMessageToStream if it’s not a stable import) alongside
user?.userId and sessionId, and change console.error prefixes to the required
pattern (e.g., "[chat/send] Cannot send message..." and "[chat/send] Failed to
send message:") so the function sendMessageToStream, user?.userId, streamsUrl,
and sessionId are all referenced correctly.
In `@apps/streams/fly.toml`:
- Around line 13-19: The app is currently allowed to auto-scale which will
create multiple machines and cause file-backed logs to diverge; update the Fly
config by disabling automatic starting so only a single instance runs: change
the auto_start_machines setting to false (auto_start_machines = false) and
ensure min_machines_running stays 1, or alternatively enforce a single instance
via an explicit runtime command (e.g., run fly scale count 1) until you have
shared storage for logs.
In `@packages/ai-chat/src/stream/client.ts`:
- Around line 205-222: The connect flow currently sets this._isConnected = true
before calling this._announcePresence(), so if announce fails the client remains
marked connected; change the logic so that this._isConnected is only set to true
after this._announcePresence() completes successfully (i.e., move the assignment
below the await this._announcePresence()), or alternatively reset
this._isConnected = false inside the catch block; update the connect method
(referencing this._isConnected and this._announcePresence) accordingly and
ensure the catch still sets this._error, calls
this._setConnectionStatus("error") and this.options.onError.
- Around line 323-331: The presence update catch handlers call
this.options.onError ?? console.error which logs without the required
"[domain/operation]" prefix; update the calls around _removePresence and
_announcePresence to use a small helper (e.g., ensurePrefixedLogger or
wrapOnError) that returns this.options.onError if present or a prefixed function
that calls console.error with a consistent "[presence/<action>]" prefix and the
error; replace all occurrences of .catch(this.options.onError ?? console.error)
in the user-presence branch (the block invoking _removePresence and
_announcePresence) with .catch(wrapOnError('presence/<operation>')) so every
fallback log is prefixed.
🧹 Nitpick comments (12)
apps/desktop/src/renderer/routes/_authenticated/_dashboard/chats/$chatId/page.tsx (1)
29-37: Navigation proceeds even when stream creation fails.If
createStreamthrows, the error is logged but navigation still occurs on line 36. This means the user lands on a chat page where the stream may not be properly initialized. Consider either blocking navigation on failure or providing user feedback.Note: The
useEffecton page load will attempt to recreate the stream, which provides some recovery, but this behavior may be intentional. If so, a comment clarifying this fallback pattern would help future maintainers.💡 Option: Block navigation on failure
const handleCreateChat = useCallback(async () => { const session = createSession(); try { await createStream(env.NEXT_PUBLIC_STREAMS_URL, session.id); + navigate({ to: "/chats/$chatId", params: { chatId: session.id } }); } catch (err) { console.error("[chats] Failed to create stream:", err); + // Optionally: show toast/notification to user + return; } - navigate({ to: "/chats/$chatId", params: { chatId: session.id } }); }, [navigate, createSession]);apps/desktop/scripts/upgrade-claude-binary.ts (2)
25-31: Use a params object forrunto follow the 2+ args rule.This helper takes two parameters; switch to a single params object to avoid positional arguments.
Proposed refactor
-function run(cmd: string, opts?: { cwd?: string }): string { - console.log(`$ ${cmd}`); - return execSync(cmd, { - encoding: "utf-8", - cwd: opts?.cwd ?? DESKTOP_DIR, - stdio: ["inherit", "pipe", "inherit"], - }).trim(); -} +function run(params: { cmd: string; cwd?: string }): string { + const { cmd, cwd } = params; + console.log(`$ ${cmd}`); + return execSync(cmd, { + encoding: "utf-8", + cwd: cwd ?? DESKTOP_DIR, + stdio: ["inherit", "pipe", "inherit"], + }).trim(); +} @@ -run(`bun run scripts/download-claude-binary.ts${versionFlag}`); +run({ cmd: `bun run scripts/download-claude-binary.ts${versionFlag}` }); @@ -run("git add resources/bin/darwin-arm64/claude"); +run({ cmd: "git add resources/bin/darwin-arm64/claude" }); @@ -const status = run("git status --short resources/bin/darwin-arm64/"); +const status = run({ cmd: "git status --short resources/bin/darwin-arm64/" });As per coding guidelines, “Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.”
Also applies to: 56-68
26-80: Prefix all logging with the required[domain/operation]pattern.Every
console.log/console.errorhere is unprefixed. Introduce a small logger helper and use it consistently.Proposed refactor
-const SCRIPT_DIR = import.meta.dirname; +const SCRIPT_DIR = import.meta.dirname; const DESKTOP_DIR = dirname(SCRIPT_DIR); const BIN_DIR = join(DESKTOP_DIR, "resources", "bin"); +const LOG_PREFIX = "[desktop/upgrade-claude-binary]"; +const log = (message: string) => console.log(`${LOG_PREFIX} ${message}`); +const logError = (message: string, error?: unknown) => + console.error(`${LOG_PREFIX} ${message}`, error); -function run(params: { cmd: string; cwd?: string }): string { +function run(params: { cmd: string; cwd?: string }): string { const { cmd, cwd } = params; - console.log(`$ ${cmd}`); + log(`$ ${cmd}`); return execSync(cmd, { encoding: "utf-8", cwd: cwd ?? DESKTOP_DIR, stdio: ["inherit", "pipe", "inherit"], }).trim(); } @@ - console.log("Claude Code Binary Upgrader"); - console.log("===========================\n"); + log("Claude Code Binary Upgrader"); + log("===========================\n"); @@ - console.log(`Current version: ${currentVersion}`); + log(`Current version: ${currentVersion}`); } else { - console.log("No existing binaries found"); + log("No existing binaries found"); } @@ - console.log("\nDownloading binary for darwin-arm64...\n"); + log("\nDownloading binary for darwin-arm64...\n"); @@ - console.log(`\nNew version: ${newVersion}`); + log(`\nNew version: ${newVersion}`); @@ - console.log("\nStaging binary in git..."); + log("\nStaging binary in git..."); @@ - console.log("\nGit status:"); + log("\nGit status:"); @@ - console.log(status || " (no changes)"); + log(status || " (no changes)"); @@ - console.log("\n✓ Upgrade complete!"); - console.log("\nNext steps:"); - console.log( + log("\n✓ Upgrade complete!"); + log("\nNext steps:"); + log( ` git commit -m "chore: upgrade claude binary to v${newVersion}"`, ); - console.log(" git push"); + log(" git push"); @@ -main().catch((error) => { - console.error("Fatal error:", error); - process.exit(1); -}); +main().catch((error) => { + logError("Fatal error:", error); + process.exit(1); +});As per coding guidelines, “Use prefixed console logging with pattern
[domain/operation] messagefor all logging.”apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx (1)
163-196: Auto-start won't retry on failure.Setting
hasAutoStarted.current = truebefore the async mutation completes means ifstartSessionfails, the component will never retry auto-starting—even on a fresh mount with the same or new sessionId.Consider resetting the flag on error to allow retry:
Suggested improvement
hasAutoStarted.current = true; console.log( `[ChatPane] Auto-starting session ${sessionId} in ${mostRecentWorkspacePath}`, ); startSessionMutation .mutateAsync({ sessionId, cwd: mostRecentWorkspacePath }) .then(() => { console.log(`[ChatPane] Session started successfully`); return refetchIsActive(); }) - .catch((err) => - console.error("[ChatPane] Failed to auto-start session:", err), - ); + .catch((err) => { + console.error("[ChatPane] Failed to auto-start session:", err); + hasAutoStarted.current = false; // Allow retry on subsequent renders + });apps/streams/src/session-registry.ts (2)
17-22: TightenSessionInfotyping for critical IDs and optional fields.
sessionIdis a critical identifier—consider a branded type to reduce accidental mix-ups, and either makecreatedByrequired or document why it’s optional.
As per coding guidelines, Avoid primitive obsession by considering branded types or wrapper objects for critical IDs.
As per coding guidelines, Make dependencies required in interfaces unless truly optional; document why if optional.
67-67: Extract JSON indentation to a named constant.
2is a magic number here; pull it up to a module-level constant to align with style rules.
As per coding guidelines, Avoid magic numbers by extracting them to named constants at module top.♻️ Proposed refactor
+const SESSIONS_JSON_INDENT = 2; + const data = JSON.stringify(Array.from(this.sessions.values()), null, 2); +const data = JSON.stringify( + Array.from(this.sessions.values()), + null, + SESSIONS_JSON_INDENT, +);apps/desktop/src/renderer/stores/tabs/types.ts (1)
105-105: Prefer a single params object for addChatPane.
This keeps the store API aligned with the params-object guideline and makes future option growth safer. Consider foldingworkspaceIdinto the options object and updating callers (including the store implementation).♻️ Suggested shape
-addChatPane: (workspaceId: string, options: AddChatPaneOptions) => string; +addChatPane: ( + params: { workspaceId: string } & AddChatPaneOptions, +) => string;As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/stores/tabs/store.ts (1)
661-737: Refactor addChatPane to accept a single params object.
This aligns the store API with the params-object guideline. Update the TabsStore interface and callers to pass{ workspaceId, sessionId, name }.♻️ Suggested signature update
-addChatPane: (workspaceId: string, options: AddChatPaneOptions) => { +addChatPane: ( + { workspaceId, ...options }: { workspaceId: string } & AddChatPaneOptions, +) => {As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/renderer/stores/tabs/utils.ts (1)
177-245: Refactor createChatPane to accept a single params object.
This keeps utility APIs consistent with the params‑object guideline and avoids positional coupling. Update callers (e.g., the tabs store) to pass{ tabId, sessionId, name }.♻️ Suggested signature update
-export const createChatPane = ( - tabId: string, - options: CreateChatPaneOptions, -): Pane => { +export const createChatPane = ( + { tabId, sessionId, name }: { tabId: string } & CreateChatPaneOptions, +): Pane => { const id = generateId("pane"); const chat: ChatState = { - sessionId: options.sessionId, + sessionId, }; return { id, tabId, type: "chat", - name: options.name ?? "Chat", + name: name ?? "Chat", chat, }; };As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts (2)
111-116: Prefer a params object for theonNewUserMessagecallback.♻️ Suggested refactor
- private onNewUserMessage: (messageId: string, content: string) => void; + private onNewUserMessage: (args: { + messageId: string; + content: string; + }) => void; - constructor(onNewUserMessage: (messageId: string, content: string) => void) { + constructor( + onNewUserMessage: (args: { messageId: string; content: string }) => void, + ) { this.onNewUserMessage = onNewUserMessage; } @@ - this.onNewUserMessage(key, content); + this.onNewUserMessage({ messageId: key, content }); @@ - const watcher = new StreamWatcher((messageId, content) => { + const watcher = new StreamWatcher(({ messageId, content }) => {As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 200-204, 291-299
131-134: Extract the polling interval into a named constant.🧹 Suggested refactor
const DURABLE_STREAM_URL = process.env.DURABLE_STREAM_URL || "http://localhost:8080"; +const STREAM_POLL_INTERVAL_MS = 500; @@ - this.intervalId = setInterval(() => this.poll(), 500); + this.intervalId = setInterval(() => this.poll(), STREAM_POLL_INTERVAL_MS);As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
apps/mobile/screens/(authenticated)/chat/[sessionId]/ChatScreen.tsx (1)
91-98: Extract scroll delay/padding constants and clear the timeout.This avoids magic numbers and prevents a pending timeout from firing after unmount.
🛠️ Proposed fix
+const SCROLL_DELAY_MS = 100; +const MESSAGE_LIST_PADDING_Y = 16; + // Scroll to bottom when new messages arrive useEffect(() => { if (messages.length > 0 || streamingMessage) { - setTimeout(() => { + const timer = setTimeout(() => { flatListRef.current?.scrollToEnd({ animated: true }); - }, 100); + }, SCROLL_DELAY_MS); + return () => clearTimeout(timer); } }, [messages.length, streamingMessage?.content, streamingMessage]); @@ contentContainerStyle={{ paddingVertical: 16 }} + contentContainerStyle={{ paddingVertical: MESSAGE_LIST_PADDING_Y }}As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
Also applies to: 192-192
| let stream: DurableStream; | ||
| try { | ||
| stream = await DurableStream.create(streamOpts); | ||
| } catch { | ||
| // Stream may already exist — connect to it | ||
| stream = await DurableStream.connect(streamOpts); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Locate the file and verify it exists
fd -t f "session-manager.ts" apps/desktop/Repository: superset-sh/superset
Length of output: 146
🏁 Script executed:
# Read the file around lines 76-82 to verify the code and context
if [ -f "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts" ]; then
wc -l "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts"
fiRepository: superset-sh/superset
Length of output: 150
🏁 Script executed:
# Get the specific code section
if [ -f "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts" ]; then
sed -n '60,95p' "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts"
fiRepository: superset-sh/superset
Length of output: 1173
🏁 Script executed:
# Search for DurableStream class definition and its create/connect methods
rg -A 10 "class DurableStream" --type ts --type tsxRepository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Check if there's error handling or logging elsewhere in this function
rg -B 5 "DurableStream.create" --type ts --type tsx -A 15Repository: superset-sh/superset
Length of output: 91
🏁 Script executed:
# Search for DurableStream class definition
rg -A 10 "class DurableStream" --type tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check error semantics of DurableStream.create method
rg "DurableStream.*create" -A 15 --type tsRepository: superset-sh/superset
Length of output: 1816
🏁 Script executed:
# Search for DurableStream definition or import
rg "DurableStream" --type ts -B 2 -A 2 | head -60Repository: superset-sh/superset
Length of output: 4431
🏁 Script executed:
# Check the full createProducer function to see all error handling context
sed -n '68,95p' "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts"Repository: superset-sh/superset
Length of output: 899
🏁 Script executed:
# Check if there's any error handling further up the call stack in the calling function
rg -B 5 "await createProducer" "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts" -A 10Repository: superset-sh/superset
Length of output: 475
🏁 Script executed:
# Verify the file path and check if it's in an apps/desktop directory (applies to tRPC guideline)
ls -la "apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/" | head -20Repository: superset-sh/superset
Length of output: 305
Log the DurableStream.create error before falling back to connect.
The catch block silently swallows the error without any logging, losing context if the create operation fails. Per coding guidelines, errors must be logged with context at minimum.
🛠️ Suggested change
- try {
- stream = await DurableStream.create(streamOpts);
- } catch {
- // Stream may already exist — connect to it
- stream = await DurableStream.connect(streamOpts);
- }
+ try {
+ stream = await DurableStream.create(streamOpts);
+ } catch (error) {
+ console.warn(
+ `[durable-stream/create] Failed to create stream ${sessionId}, falling back to connect:`,
+ error,
+ );
+ stream = await DurableStream.connect(streamOpts);
+ }🤖 Prompt for AI Agents
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`
around lines 76 - 82, The catch block that falls back from DurableStream.create
to DurableStream.connect is silently swallowing the error; update the catch to
log the caught error with context (including streamOpts) before attempting
DurableStream.connect so failures are visible; locate the try/catch around
DurableStream.create in session-manager.ts and add a processLogger (or the
existing logger) call that includes a descriptive message, the error object, and
relevant identifiers prior to calling DurableStream.connect(streamOpts).
| {messages.map((msg, index) => { | ||
| if ("type" in msg && msg.type === "streaming") { | ||
| return ( | ||
| <ChatMessage | ||
| key="streaming" | ||
| content={msg.content} | ||
| contentBlocks={msg.contentBlocks} | ||
| toolResults={msg.toolResults} | ||
| isStreaming | ||
| /> |
There was a problem hiding this comment.
Ensure unique keys for streaming rows.
Line 54 uses a constant key ("streaming"). If two streaming entries ever coexist (or a message ID collides), React may reuse the wrong component. Use a unique key per row.
✅ Suggested fix
- key="streaming"
+ key={`streaming-${index}`}🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/chats/components/ChatMessageList/ChatMessageList.tsx`
around lines 50 - 59, The Streaming message row uses a constant key
("streaming") inside the messages.map rendering, which can cause React to reuse
components incorrectly; update the key prop on the ChatMessage returned in the
messages.map branch (where "type" in msg && msg.type === "streaming") to be
unique per item — for example combine a unique identifier from the message
(msg.id or similar) or fallback to the map index (e.g., `streaming-${index}`) so
each streaming row has a distinct key.
| const handleClick = useCallback(async () => { | ||
| const session = createSession(); | ||
| await createStream(STREAM_SERVER_URL, session.id); | ||
| navigate({ to: "/chats/$chatId", params: { chatId: session.id } }); |
There was a problem hiding this comment.
Handle createStream failures so navigation doesn’t silently fail.
If createStream rejects (network/server down), the click handler aborts before navigation and the error lacks contextual logging. Please catch and log with the required prefix, and decide whether to navigate or surface an error state.
🧯 Suggested fix (with prefixed logging)
- const handleClick = useCallback(async () => {
- const session = createSession();
- await createStream(STREAM_SERVER_URL, session.id);
- navigate({ to: "/chats/$chatId", params: { chatId: session.id } });
- }, [navigate, createSession]);
+ const handleClick = useCallback(async () => {
+ const session = createSession();
+ try {
+ await createStream(STREAM_SERVER_URL, session.id);
+ navigate({ to: "/chats/$chatId", params: { chatId: session.id } });
+ } catch (error) {
+ console.error("[ai-chat/createStream] failed to open stream", error);
+ // Optionally surface a toast or inline error state here
+ }
+ }, [navigate, createSession]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleClick = useCallback(async () => { | |
| const session = createSession(); | |
| await createStream(STREAM_SERVER_URL, session.id); | |
| navigate({ to: "/chats/$chatId", params: { chatId: session.id } }); | |
| const handleClick = useCallback(async () => { | |
| const session = createSession(); | |
| try { | |
| await createStream(STREAM_SERVER_URL, session.id); | |
| navigate({ to: "/chats/$chatId", params: { chatId: session.id } }); | |
| } catch (error) { | |
| console.error("[ai-chat/createStream] failed to open stream", error); | |
| // Optionally surface a toast or inline error state here | |
| } | |
| }, [navigate, createSession]); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/routes/_authenticated/_dashboard/components/TopBar/components/ChatInputButton/ChatInputButton.tsx`
around lines 15 - 18, The click handler handleClick should catch failures from
createStream so navigation doesn't silently abort: wrap the await
createStream(STREAM_SERVER_URL, session.id) call in a try/catch inside the
useCallback for handleClick, log the error with a contextual prefix like
"[chat/createStream]" plus the error details, and then decide whether to proceed
with navigate({ to: "/chats/$chatId", params: { chatId: session.id } }) or
surface an error state (e.g., set an error flag or show a toast) instead of
swallowing the exception; keep references to createSession, createStream and
navigate when implementing the change.
| const handleAddChat = useCallback(async () => { | ||
| if (!activeWorkspaceId) return; | ||
| const session = createSession(); | ||
| await createStream(STREAM_SERVER_URL, session.id); | ||
| addChatPane(activeWorkspaceId, { | ||
| sessionId: session.id, | ||
| name: session.name, | ||
| }); | ||
| }, [activeWorkspaceId, createSession, addChatPane]); |
There was a problem hiding this comment.
Missing error handling leaves inconsistent state on failure.
If createStream throws, the session is already persisted in the store (and synced to the streams server), but the chat pane is never added. The user sees no feedback, and the app is left in an inconsistent state with an orphaned session.
🛡️ Proposed fix with error handling
const handleAddChat = useCallback(async () => {
if (!activeWorkspaceId) return;
- const session = createSession();
- await createStream(STREAM_SERVER_URL, session.id);
- addChatPane(activeWorkspaceId, {
- sessionId: session.id,
- name: session.name,
- });
+ const session = createSession();
+ try {
+ await createStream(STREAM_SERVER_URL, session.id);
+ addChatPane(activeWorkspaceId, {
+ sessionId: session.id,
+ name: session.name,
+ });
+ } catch (error) {
+ console.error("[chat/create] Failed to create stream:", error);
+ // Consider: delete the orphaned session or show user feedback
+ }
}, [activeWorkspaceId, createSession, addChatPane]);As per coding guidelines: "Never swallow errors silently; at minimum log them with context" and "Use prefixed console logging with pattern [domain/operation] message for all logging".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const handleAddChat = useCallback(async () => { | |
| if (!activeWorkspaceId) return; | |
| const session = createSession(); | |
| await createStream(STREAM_SERVER_URL, session.id); | |
| addChatPane(activeWorkspaceId, { | |
| sessionId: session.id, | |
| name: session.name, | |
| }); | |
| }, [activeWorkspaceId, createSession, addChatPane]); | |
| const handleAddChat = useCallback(async () => { | |
| if (!activeWorkspaceId) return; | |
| const session = createSession(); | |
| try { | |
| await createStream(STREAM_SERVER_URL, session.id); | |
| addChatPane(activeWorkspaceId, { | |
| sessionId: session.id, | |
| name: session.name, | |
| }); | |
| } catch (error) { | |
| console.error("[chat/create] Failed to create stream:", error); | |
| // Consider: delete the orphaned session or show user feedback | |
| } | |
| }, [activeWorkspaceId, createSession, addChatPane]); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/GroupStrip/GroupStrip.tsx`
around lines 101 - 109, handleAddChat can leave an orphaned session if
createStream throws; wrap the createStream + addChatPane sequence in a
try/catch, and on error log with the prefixed pattern (e.g.
console.error(`[streams/create] Failed to create stream for session
${session.id}:`, err)) and perform rollback (remove the persisted session using
your session deletion API/function—e.g. removeSession(session.id) or
deleteSession(session.id)) so the store stays consistent, and surface user
feedback or rethrow as appropriate; reference symbols: handleAddChat,
createSession, createStream, addChatPane, STREAM_SERVER_URL, and the session.id
returned by createSession.
| const handleSend = useCallback( | ||
| async (content: string) => { | ||
| if (!user?.userId || !streamsUrl || !sessionId) { | ||
| console.error( | ||
| "[chat] Cannot send message: missing user, URL, or sessionId", | ||
| ); | ||
| return; | ||
| } | ||
| try { | ||
| await sendMessageToStream({ | ||
| proxyUrl: streamsUrl, | ||
| sessionId, | ||
| userId: user.userId, | ||
| content, | ||
| }); | ||
| } catch (err) { | ||
| console.error("[chat] Failed to send message:", err); | ||
| } | ||
| }, | ||
| [user?.userId, sessionId], | ||
| ); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n apps/mobile/screens/\(authenticated\)/chat/\[sessionId\]/ChatScreen.tsx | sed -n '95,125p'Repository: superset-sh/superset
Length of output: 994
🏁 Script executed:
find . -name "ChatScreen.tsx" -type fRepository: superset-sh/superset
Length of output: 133
🏁 Script executed:
cat -n apps/mobile/screens/\(authenticated\)/chat/\[sessionId\]/ChatScreen.tsx | head -100 | tail -60Repository: superset-sh/superset
Length of output: 1913
Fix hook dependency array and log prefixes in handleSend.
streamsUrl is used in the callback but missing from the dependency array. Log prefixes should follow the [domain/operation] pattern per coding guidelines.
🛠️ Proposed fix
if (!user?.userId || !streamsUrl || !sessionId) {
console.error(
- "[chat] Cannot send message: missing user, URL, or sessionId",
+ "[chat/send] Cannot send message: missing user, URL, or sessionId",
);
return;
}
@@
} catch (err) {
- console.error("[chat] Failed to send message:", err);
+ console.error("[chat/send] Failed to send message:", err);
}
},
- [user?.userId, sessionId],
+ [user?.userId, sessionId, streamsUrl],
);🤖 Prompt for AI Agents
In `@apps/mobile/screens/`(authenticated)/chat/[sessionId]/ChatScreen.tsx around
lines 100 - 120, The handleSend callback is missing streamsUrl (and the
sendMessageToStream reference if not stable) in its dependency array and uses
the wrong log prefix; update the dependency array for handleSend to include
streamsUrl (and sendMessageToStream if it’s not a stable import) alongside
user?.userId and sessionId, and change console.error prefixes to the required
pattern (e.g., "[chat/send] Cannot send message..." and "[chat/send] Failed to
send message:") so the function sendMessageToStream, user?.userId, streamsUrl,
and sessionId are all referenced correctly.
| [http_service] | ||
| internal_port = 8080 | ||
| force_https = true | ||
| auto_stop_machines = "stop" | ||
| auto_start_machines = true | ||
| min_machines_running = 1 | ||
| processes = ["app"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Check if the file exists and its full content
cd apps/streams 2>/dev/null && cat -n fly.toml || echo "File not found in apps/streams"Repository: superset-sh/superset
Length of output: 906
🏁 Script executed:
# Search for DATA_DIR references in the codebase to understand file-backed streams
rg "DATA_DIR" --type js --type ts --type sh -iRepository: superset-sh/superset
Length of output: 459
🌐 Web query:
Fly.io max_machines_running fly.toml http_service configuration
💡 Result:
In Fly.io, there is no max_machines_running setting in fly.toml (including under [http_service]). Fly Proxy autostop/autostart never creates or destroys Machines, so the maximum number of running Machines is simply the number of Machines that exist for the app. To “cap” how many can run, only create/keep that many Machines (for example, via fly scale count). [1], [2]
What you can configure in fly.toml under [http_service] (or [[services]]) is autostop/autostart and a minimum in the primary region, e.g.:
auto_stop_machines = "stop" | "suspend" | "off"auto_start_machines = true|falsemin_machines_running = <n>(primary region only; requiresauto_stop_machines="stop"or"suspend") [1], [2]
If you need more sophisticated scaling behavior, Fly’s docs point to using an autoscaler approach (e.g., fly-autoscaler) in addition to Proxy autostart/autostop. [3]
Sources: [1] [2] [3]
Pin to a single machine while streams are file-backed.
If more than one machine runs, file-backed logs will diverge and break real-time sync. The current configuration risks scaling beyond one machine. Set auto_start_machines = false or use fly scale count 1 to enforce single-machine operation until shared storage is in place.
🤖 Prompt for AI Agents
In `@apps/streams/fly.toml` around lines 13 - 19, The app is currently allowed to
auto-scale which will create multiple machines and cause file-backed logs to
diverge; update the Fly config by disabling automatic starting so only a single
instance runs: change the auto_start_machines setting to false
(auto_start_machines = false) and ensure min_machines_running stays 1, or
alternatively enforce a single instance via an explicit runtime command (e.g.,
run fly scale count 1) until you have shared storage for logs.
| try { | ||
| this._setConnectionStatus("connecting"); | ||
|
|
||
| // Preload stream data | ||
| await this._db.preload(); | ||
|
|
||
| this._isConnected = true; | ||
| this._setConnectionStatus("connected"); | ||
|
|
||
| // Announce presence if we have a user | ||
| if (this.options.user) { | ||
| await this._announcePresence(); | ||
| } | ||
| } catch (error) { | ||
| this._error = error instanceof Error ? error : new Error(String(error)); | ||
| this._setConnectionStatus("error"); | ||
| this.options.onError?.(this._error); | ||
| throw error; |
There was a problem hiding this comment.
Avoid leaving _isConnected true when presence fails.
If _announcePresence() throws, connect() marks status as "error" but _isConnected stays true, which prevents reconnect and can strand the UI in an error state even though the stream may still be usable. Consider setting _isConnected only after presence succeeds (or resetting it in the catch).
🛠️ Proposed fix
// Preload stream data
await this._db.preload();
- this._isConnected = true;
- this._setConnectionStatus("connected");
-
// Announce presence if we have a user
if (this.options.user) {
await this._announcePresence();
}
+
+ this._isConnected = true;
+ this._setConnectionStatus("connected");
} catch (error) {
+ this._isConnected = false;
this._error = error instanceof Error ? error : new Error(String(error));
this._setConnectionStatus("error");
this.options.onError?.(this._error);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| try { | |
| this._setConnectionStatus("connecting"); | |
| // Preload stream data | |
| await this._db.preload(); | |
| this._isConnected = true; | |
| this._setConnectionStatus("connected"); | |
| // Announce presence if we have a user | |
| if (this.options.user) { | |
| await this._announcePresence(); | |
| } | |
| } catch (error) { | |
| this._error = error instanceof Error ? error : new Error(String(error)); | |
| this._setConnectionStatus("error"); | |
| this.options.onError?.(this._error); | |
| throw error; | |
| try { | |
| this._setConnectionStatus("connecting"); | |
| // Preload stream data | |
| await this._db.preload(); | |
| // Announce presence if we have a user | |
| if (this.options.user) { | |
| await this._announcePresence(); | |
| } | |
| this._isConnected = true; | |
| this._setConnectionStatus("connected"); | |
| } catch (error) { | |
| this._isConnected = false; | |
| this._error = error instanceof Error ? error : new Error(String(error)); | |
| this._setConnectionStatus("error"); | |
| this.options.onError?.(this._error); | |
| throw error; |
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/client.ts` around lines 205 - 222, The connect
flow currently sets this._isConnected = true before calling
this._announcePresence(), so if announce fails the client remains marked
connected; change the logic so that this._isConnected is only set to true after
this._announcePresence() completes successfully (i.e., move the assignment below
the await this._announcePresence()), or alternatively reset this._isConnected =
false inside the catch block; update the connect method (referencing
this._isConnected and this._announcePresence) accordingly and ensure the catch
still sets this._error, calls this._setConnectionStatus("error") and
this.options.onError.
| this._removePresence().catch(this.options.onError ?? console.error); | ||
| } else if (user && !previousUser) { | ||
| // User logged in - announce presence | ||
| this._announcePresence().catch(this.options.onError ?? console.error); | ||
| } else if (user && previousUser && user.userId !== previousUser.userId) { | ||
| // User changed - update presence | ||
| this._removePresence() | ||
| .then(() => this._announcePresence()) | ||
| .catch(this.options.onError ?? console.error); |
There was a problem hiding this comment.
Prefix fallback console logging in presence updates.
When onError isn’t provided, .catch(this.options.onError ?? console.error) logs without the required [domain/operation] prefix. Consider a small helper to ensure consistent, prefixed logging.
🛠️ Proposed fix
setUser(user: SessionUser | null): void {
const previousUser = this.options.user;
(this.options as { user: SessionUser | null }).user = user;
+ const logPresenceError =
+ (operation: string) => (err: unknown) => {
+ const error = err instanceof Error ? err : new Error(String(err));
+ if (this.options.onError) {
+ this.options.onError(error);
+ } else {
+ console.error(
+ `[ai-chat/${operation}] Failed to update presence:`,
+ error,
+ );
+ }
+ };
+
// If connected, update presence
if (this._isConnected) {
if (previousUser && !user) {
// User logged out - remove presence
- this._removePresence().catch(this.options.onError ?? console.error);
+ this._removePresence().catch(logPresenceError("remove-presence"));
} else if (user && !previousUser) {
// User logged in - announce presence
- this._announcePresence().catch(this.options.onError ?? console.error);
+ this._announcePresence().catch(logPresenceError("announce-presence"));
} else if (user && previousUser && user.userId !== previousUser.userId) {
// User changed - update presence
this._removePresence()
.then(() => this._announcePresence())
- .catch(this.options.onError ?? console.error);
+ .catch(logPresenceError("switch-presence"));
}
}
}As per coding guidelines: Use prefixed console logging with pattern [domain/operation] message for all logging.
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/client.ts` around lines 323 - 331, The presence
update catch handlers call this.options.onError ?? console.error which logs
without the required "[domain/operation]" prefix; update the calls around
_removePresence and _announcePresence to use a small helper (e.g.,
ensurePrefixedLogger or wrapOnError) that returns this.options.onError if
present or a prefixed function that calls console.error with a consistent
"[presence/<action>]" prefix and the error; replace all occurrences of
.catch(this.options.onError ?? console.error) in the user-presence branch (the
block invoking _removePresence and _announcePresence) with
.catch(wrapOnError('presence/<operation>')) so every fallback log is prefixed.
…essages V2 SDK (unstable_v2_createSession) did not emit stream_event messages, had no cwd support, and lacked proper session resume. Switched to V1 query() API which supports includePartialMessages for real-time token streaming, cwd for correct workspace directory, and resume for multi-turn conversations. Also fixes: - Loading state gap by replacing isSending with isAwaitingResponse - Ghost streaming messages from lone result chunks in materialize - Per-message producer flushing for real-time visibility - Debug logging across the full streaming pipeline
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx`:
- Line 202: The console.log in ChatPane.tsx that prints message text (e.g., the
line starting with console.log(`[ChatPane] Sending message:`) and the similar
logs around lines 249-251) must stop outputting user content; replace those logs
to only emit non-sensitive metadata (message length, message id, user id, or a
debug-only flag) and/or gate them behind an explicit debug flag (e.g., only log
when isDev or DEBUG is true). Locate the logging in the ChatPane component’s
send/stream handlers and update the messages so they never include message
content or chunks—log only safe metadata and ensure the debug gating is
consulted before emitting anything.
- Around line 203-211: The current flow in ChatPane clears the user's draft
before awaiting sendMessage, causing message loss if sendMessage throws;
instead, setIsAwaitingResponse(true) before sending, await sendMessage, and only
then call setDraft("") on success (inside the try after await). In the catch
block leave the draft intact (or restore the previous content) and ensure
setIsAwaitingResponse(false) runs on failure; update references to sendMessage,
setDraft, setIsAwaitingResponse, and the ChatPane send handler accordingly.
In `@packages/ai-chat/src/stream/useChatSession.ts`:
- Around line 214-221: The useMemo that computes draft (function named draft
using useMemo) currently only depends on draftRows but also reads
clientOptions.user, so update the dependency array to include clientOptions.user
(or clientOptions) so the memo recalculates when the user changes; locate the
useMemo that references StreamDraft and myDraft and add the missing dependency
to the array.
🧹 Nitpick comments (7)
packages/ai-chat/src/stream/client.ts (2)
291-313: Silent early return hides missing user context.
updateDraftsilently returns without logging when no user is present. While this may be intentional for fire-and-forget usage, consider adding debug logging to aid troubleshooting.🔧 Suggested improvement
async updateDraft(content: string): Promise<void> { const user = this.options.user; - if (!user) return; + if (!user) { + console.debug("[ai-chat/client] updateDraft skipped: no user"); + return; + }
380-393: Addcredentials: "include"to the fetch call for cross-origin authentication.The codebase consistently uses
credentials: "include"in fetch calls across TRPC clients (apps/web, apps/admin) and other API interactions. The_appendToStreammethod should follow this pattern to ensure authentication cookies are sent when the proxy URL points to a different origin. This affects other fetch calls in the ai-chat package as well (e.g., inactions.ts).credentials: "include",packages/ai-chat/src/stream/useCollectionData.ts (1)
55-103: Refs updated during render may cause issues in React Strict Mode.Updating
subscribeRef.currentandgetSnapshotRef.currentduring render (lines 56-63, 83-94) is an anti-pattern that can cause issues with React's concurrent features. While this appears to be copied from Electric SQL's implementation, the standard pattern is to useuseCallbackoruseEffectfor updates.However, since this is explicitly noted as "copied verbatim" and the pattern works for the specific use case of tracking collection identity changes, this is acceptable for now.
Consider refactoring to use
useCallbackwith the collection as a dependency if you encounter issues with React Strict Mode or concurrent rendering:const subscribe = useCallback( (onStoreChange: () => void): (() => void) => { const subscription = collection.subscribeChanges(() => { versionRef.current++; console.log(`[ai-chat/collection] change detected, version=${versionRef.current}`); onStoreChange(); }); return () => subscription.unsubscribe(); }, [collection] );apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx (2)
34-59: Prefer params objects for multi-arg callbacks.
splitPaneAuto,splitPaneHorizontal,splitPaneVertical, andsetFocusedPaneuse multiple positional parameters; please switch to a single params object to avoid positional confusion and align with project conventions.As per coding guidelines, Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
71-98: ExtractChatMessageBlockinto its own component file.This file sits under
components/yet defines more than one component. Please moveChatMessageBlocktoChatMessageBlock/ChatMessageBlock.tsxand export via anindex.tsbarrel.As per coding guidelines, Use folder structure with one component per file:
ComponentName/ComponentName.tsxwith barrel export inindex.ts.apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts (2)
113-117: Use a params object foronNewUserMessage.
Two positional args violate the local guideline; a named object is clearer.♻️ Suggested change
- private onNewUserMessage: (messageId: string, content: string) => void; + private onNewUserMessage: (args: { messageId: string; content: string }) => void; - constructor(onNewUserMessage: (messageId: string, content: string) => void) { + constructor( + onNewUserMessage: (args: { messageId: string; content: string }) => void, + ) { this.onNewUserMessage = onNewUserMessage; }- this.onNewUserMessage(key, content); + this.onNewUserMessage({ messageId: key, content });As per coding guidelines: Functions with 2+ parameters should accept a single params object with named properties instead of positional arguments.
Also applies to: 202-205
133-133: Extract the poll interval to a named constant.
This avoids a magic number and makes tuning easier.♻️ Suggested change
+const STREAM_POLL_INTERVAL_MS = 500; + ... - this.intervalId = setInterval(() => this.poll(), 500); + this.intervalId = setInterval(() => this.poll(), STREAM_POLL_INTERVAL_MS);As per coding guidelines: Avoid magic numbers by extracting them to named constants at module top.
|
|
||
| const handleSend = useCallback( | ||
| async (content: string) => { | ||
| console.log(`[ChatPane] Sending message: "${content.slice(0, 50)}..."`); |
There was a problem hiding this comment.
Remove/redact message content from logs.
Logging user message content (even truncated) and streaming content can leak sensitive data. Log only metadata (e.g., length, ids) or gate behind debug-only flags.
Proposed fix
- console.log(`[ChatPane] Sending message: "${content.slice(0, 50)}..."`);
+ console.log(`[ChatPane] Sending message (len=${content.length})`);
@@
- `[ChatPane] Building allMessages: messages=${messages.length} streamingMessage=${streamingMessage ? `id=${streamingMessage.id.slice(0, 8)} role=${streamingMessage.role} content="${streamingMessage.content.slice(0, 80)}" blocks=${streamingMessage.contentBlocks.length} isComplete=${streamingMessage.isComplete} isStreaming=${streamingMessage.isStreaming}` : "null"}`,
+ `[ChatPane] Building allMessages: messages=${messages.length} streamingMessage=${streamingMessage ? `id=${streamingMessage.id.slice(0, 8)} role=${streamingMessage.role} contentLen=${streamingMessage.content.length} blocks=${streamingMessage.contentBlocks.length} isComplete=${streamingMessage.isComplete} isStreaming=${streamingMessage.isStreaming}` : "null"}`,Also applies to: 249-251
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx`
at line 202, The console.log in ChatPane.tsx that prints message text (e.g., the
line starting with console.log(`[ChatPane] Sending message:`) and the similar
logs around lines 249-251) must stop outputting user content; replace those logs
to only emit non-sensitive metadata (message length, message id, user id, or a
debug-only flag) and/or gate them behind an explicit debug flag (e.g., only log
when isDev or DEBUG is true). Locate the logging in the ChatPane component’s
send/stream handlers and update the messages so they never include message
content or chunks—log only safe metadata and ensure the debug gating is
consulted before emitting anything.
| setIsAwaitingResponse(true); | ||
| setDraft(""); | ||
| try { | ||
| await sendMessage(content); | ||
| console.log(`[ChatPane] Message sent successfully`); | ||
| } catch (err) { | ||
| console.error("[ChatPane] Failed to send message:", err); | ||
| setIsAwaitingResponse(false); | ||
| } |
There was a problem hiding this comment.
Don’t clear the draft before send succeeds (message loss on failure).
If sendMessage fails, the user’s input is lost. Clear the draft only after a successful send (or restore on failure).
Proposed fix
- setIsAwaitingResponse(true);
- setDraft("");
+ setIsAwaitingResponse(true);
+ const previousDraft = content;
try {
await sendMessage(content);
+ setDraft("");
console.log(`[ChatPane] Message sent successfully`);
} catch (err) {
console.error("[ChatPane] Failed to send message:", err);
setIsAwaitingResponse(false);
+ setDraft(previousDraft);
}🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx`
around lines 203 - 211, The current flow in ChatPane clears the user's draft
before awaiting sendMessage, causing message loss if sendMessage throws;
instead, setIsAwaitingResponse(true) before sending, await sendMessage, and only
then call setDraft("") on success (inside the try after await). In the catch
block leave the draft intact (or restore the previous content) and ensure
setIsAwaitingResponse(false) runs on failure; update references to sendMessage,
setDraft, setIsAwaitingResponse, and the ChatPane send handler accordingly.
| const draft = useMemo((): string => { | ||
| const user = clientOptions.user; | ||
| if (!user) return ""; | ||
| const myDraft = draftRows.find( | ||
| (d: StreamDraft) => d.userId === user.userId, | ||
| ); | ||
| return myDraft?.content ?? ""; | ||
| }, [draftRows]); |
There was a problem hiding this comment.
Missing dependency clientOptions.user in useMemo for draft.
The draft memoization depends on clientOptions.user but only lists draftRows in the dependency array. If the user changes, the draft won't recalculate until draftRows changes.
🛠️ Proposed fix
// Current user's draft
const draft = useMemo((): string => {
const user = clientOptions.user;
if (!user) return "";
const myDraft = draftRows.find(
(d: StreamDraft) => d.userId === user.userId,
);
return myDraft?.content ?? "";
- }, [draftRows]);
+ }, [draftRows, clientOptions.user]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const draft = useMemo((): string => { | |
| const user = clientOptions.user; | |
| if (!user) return ""; | |
| const myDraft = draftRows.find( | |
| (d: StreamDraft) => d.userId === user.userId, | |
| ); | |
| return myDraft?.content ?? ""; | |
| }, [draftRows]); | |
| const draft = useMemo((): string => { | |
| const user = clientOptions.user; | |
| if (!user) return ""; | |
| const myDraft = draftRows.find( | |
| (d: StreamDraft) => d.userId === user.userId, | |
| ); | |
| return myDraft?.content ?? ""; | |
| }, [draftRows, clientOptions.user]); |
🤖 Prompt for AI Agents
In `@packages/ai-chat/src/stream/useChatSession.ts` around lines 214 - 221, The
useMemo that computes draft (function named draft using useMemo) currently only
depends on draftRows but also reads clientOptions.user, so update the dependency
array to include clientOptions.user (or clientOptions) so the memo recalculates
when the user changes; locate the useMemo that references StreamDraft and
myDraft and add the missing dependency to the array.
# Conflicts: # apps/desktop/package.json # apps/marketing/src/app/team/page.tsx # bun.lock
2 participants
AI Chat: Claude Code Integration
Multi-platform Claude Code chat using durable streams for real-time sync.
Architecture
SDKMessageobjects to a durable streamapps/streams): Append-only log for real-time message delivery. Currently file-backed, deployed on Fly.iopackages/ai-chat):useChatSessionhook +materializeMessages()— reconstructs UI state (text, tool calls, thinking blocks) from raw SDK chunks/chats, renders SDK content blocks directly (markdown, collapsible tool cards, thinking)/chat/[sessionId], same shared hookWhat's done
createSession/resumeSessionai-chatPostHog feature flag (@superset.sh only)What's left
Storage Architecture (next major piece)
The durable streams container currently uses file-backed storage (LMDB + append-only logs) with a JSON file for session metadata. The target architecture:
The streams container is internal infrastructure (only the session manager writes to it), so no auth needed on it. In-memory is fine since Neon is the durable SOT. If the container restarts, clients reconnect and load history from the DB.
This means:
session-registry.ts)dataDir)Other TODOs
packages/ai-chat— many unused exports from pre-durable-streams era (types.tsis entirely dead code)lib/streams/client.tssession registry functions should move to tRPC once DB-backed sessions landTest plan
Summary by CodeRabbit