fix(desktop): restore Claude session dropdown loading + click import#1795
Closed
Kitenite wants to merge 4 commits into
Closed
fix(desktop): restore Claude session dropdown loading + click import#1795Kitenite wants to merge 4 commits into
Kitenite wants to merge 4 commits into
Conversation
Contributor
📝 WalkthroughWalkthroughAdds Claude session import: new chatServiceClaude tRPC router (listSessions, importSession), UI hooks to list/import Claude JSON/JSONL sessions from disk, a session-conversion subsystem to normalize external session formats, and UI wiring to select imported sessions. Changes
Sequence DiagramsequenceDiagram
participant User
participant UI as SessionSelector
participant Client as electronTrpc Client
participant Router as chatServiceClaude Router
participant FS as FileSystem
participant Conv as SessionConverter
User->>UI: Open dropdown / request list
UI->>Client: chatServiceClaude.listSessions(cwd, limit)
Client->>Router: listSessions request
Router->>FS: read session roots / metadata
FS-->>Router: session list
Client-->>UI: session list
User->>UI: import session (sessionId, orgId)
UI->>Client: chatServiceClaude.importSession(sessionId, orgId)
Client->>Router: importSession request
Router->>FS: stream/read session file (JSON/JSONL)
Router->>Conv: convertExternalSessionToChatChunks(entries, provider)
Conv-->>Router: normalized message chunks
Router->>Router: persist/create/dedupe chat session
Router-->>Client: import success (importedSessionId)
Client-->>UI: success response
UI->>User: select imported session / show toast
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (5)
packages/chat/src/shared/session-conversion.ts (3)
241-244:extractTimestampis called twice with the same argument on the non-default path.In both
codexSessionConverter.convert(lines 241-244) andclaudeCodeSessionConverter.convert(lines 296-299),extractTimestamp(message ?? {})is evaluated once in the condition and again in the consequent branch. Cache the result in a local variable.♻️ Example fix (codex converter, same pattern applies to claude-code)
- const createdAt = - extractTimestamp(message ?? {}) !== DEFAULT_TIMESTAMP - ? extractTimestamp(message ?? {}) - : extractTimestamp(record); + const messageTimestamp = extractTimestamp(message ?? {}); + const createdAt = + messageTimestamp !== DEFAULT_TIMESTAMP + ? messageTimestamp + : extractTimestamp(record);Also applies to: 296-299
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 241 - 244, In both codexSessionConverter.convert and claudeCodeSessionConverter.convert the call extractTimestamp(message ?? {}) is evaluated twice; cache it in a local variable (e.g., timestampFromMessage = extractTimestamp(message ?? {})) and then set createdAt using that variable: if timestampFromMessage !== DEFAULT_TIMESTAMP use it, otherwise fall back to extractTimestamp(record). Update both converters to use the cached timestampFromMessage to avoid duplicate work and ensure the fallback still calls extractTimestamp(record).
255-278:claudeCodeSessionConverter.detectis very broad — any record withmessage.roleset to a valid role will match.Because the codex converter's
detectchecks for specific discriminators (kind === "codex_event",turn_id,dir), it won't match generic records. However, the claude-codedetecton line 262-263 returnstruefor any object that hasmessage.roleset to"user","assistant", or"system". This means every codex entry (which also carriesmessage.role) will also satisfy claude-code's detector.Auto-detection still works correctly today because
resolveConverterpicks the converter with the highest score, and both converters score equally on codex entries while only codex matches its own discriminators. But this makes the scoring fragile — if entry counts ever differ, or a new converter is added, results could silently flip. Consider tightening claude-code detection (e.g., requireparentUuid/uuidor exclude entries wherekind === "codex_event").🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 255 - 278, claudeCodeSessionConverter.detect is too permissive — it returns true for any record with message.role, which causes it to collide with codex entries; tighten the detector in claudeCodeSessionConverter.detect (and keep using helpers asRecord/asString/normalizeRole/extractText) by requiring stronger discriminators (e.g., require that record.parentUuid/parent_uuid or record.uuid exists and extractText(record).length > 0) and/or explicitly exclude codex-style entries (check for kind === "codex_event" or presence of codex-specific fields like turn_id/dir and return false). Ensure the updated logic still allows legitimate Claude code sessions but prevents matching codex entries.
415-416:defaultSessionConverterRegistryis a mutable module-level singleton.Any consumer that calls
.register()or.unregister()on this instance mutates the shared registry for every other caller in the process. This is especially risky in a long-lived Electron main process. Consider makingconvertExternalSessionToChatChunkscreate a fresh registry per call, or at minimum document the shared-mutable nature prominently.♻️ Safest minimal fix: create a fresh registry per call
export function convertExternalSessionToChatChunks( options: ConvertExternalSessionOptions, ): ConvertExternalSessionResult { - return defaultSessionConverterRegistry.convert(options); + return createDefaultSessionConverterRegistry().convert(options); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 415 - 416, The module-level singleton defaultSessionConverterRegistry (created with createDefaultSessionConverterRegistry) is shared-mutable and can be mutated by callers; change convertExternalSessionToChatChunks to instantiate a fresh registry by calling createDefaultSessionConverterRegistry() inside the function (instead of using the exported defaultSessionConverterRegistry) so each call uses an independent registry, and remove or deprecate the mutable exported singleton; update places that relied on the shared instance to accept a registry parameter or rely on the per-call registry created in convertExternalSessionToChatChunks.apps/desktop/src/lib/trpc/routers/chat-service/index.ts (1)
90-143:listClaudeSessionsstats every candidate file sequentially.For large Claude project directories with many
.jsonlfiles, the sequentialstatcalls (lines 119-124) could be slow. Consider parallelizing withPromise.all(orPromise.allSettledto handle individual failures).♻️ Parallel stat
- const withStats: Array<{ filePath: string; fileStat: FileStat }> = []; - for (const filePath of candidates) { - try { - const fileStat = await stat(filePath); - withStats.push({ filePath, fileStat }); - } catch {} - } + const withStats = ( + await Promise.allSettled( + candidates.map(async (filePath) => ({ + filePath, + fileStat: await stat(filePath), + })), + ) + ) + .filter( + (r): r is PromiseFulfilledResult<{ filePath: string; fileStat: FileStat }> => + r.status === "fulfilled", + ) + .map((r) => r.value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 90 - 143, The file I/O loop in listClaudeSessions performs await stat(filePath) sequentially (the withStats population), which is slow for many files; change it to run stat calls in parallel using Promise.allSettled over candidates.map(filePath => stat(filePath)) (or Promise.all with try/catch per promise), then collect only fulfilled results and pair each resolved FileStat with its filePath to build withStats, preserving the existing sort/slice/map logic; reference listClaudeSessions, the candidates array and the withStats population so you replace the sequential try/catch loop with a parallel Promise.allSettled-based approach that filters out failures.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx (1)
148-177: All Claude session items are disabled during an import, but there's no per-item loading indicator.Line 150 disables every Claude session item while
isPendingis true, but the user has no visual feedback on which session is being imported. Consider adding a spinner or highlight to the specific item being imported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx` around lines 148 - 177, The dropdown currently disables all Claude items using importClaudeSessionMutation.isPending without indicating which specific item is importing; add a per-item loading indicator and only disable the active item by tracking the file being imported (e.g., introduce an importingSessionId state), update the onSelect handler in the DropdownMenuItem for claudeSession.filePath to set importingSessionId = claudeSession.filePath before calling importClaudeSessionMutation.mutateAsync and clear it in finally, and render a spinner or highlight inside the DropdownMenuItem when importingSessionId === claudeSession.filePath while setting disabled to importClaudeSessionMutation.isPending && importingSessionId === claudeSession.filePath so other items remain interactive.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 370-400: The listSessions endpoint is using publicProcedure
(function name: listSessions) which lacks authentication and can expose local
Claude session files via listClaudeSessions; change it to require auth the same
way importSession does (see importSession -> importClaudeSession which uses
getAuthHeaders) by switching listSessions to the protected/authenticated
procedure wrapper or explicitly validate auth (call the same getAuthHeaders or
auth-check helper before invoking listClaudeSessions) so only authenticated
users can enumerate sessions.
- Around line 108-116: The current logic sets candidates =
workspaceMatches.length > 0 ? workspaceMatches : filePaths which falls back to
all Claude sessions and can leak other projects; change it so that when
workspaceMatches is empty you return an empty array (or otherwise restrict to
workspace-only sessions) instead of using filePaths. Update the code using the
symbols workspaceMatches, candidates, workspaceProjectId and filePaths so
candidates = workspaceMatches (or [] when no matches) and ensure the
caller/return path respects that empty list result so unrelated .jsonl sessions
are not exposed.
- Around line 81-88: The isWithinDirectory check should resolve symlinks before
comparing paths: in importClaudeSession, call fs.realpath (or fs.realpathSync)
on normalizedFilePath and on claudeProjectsRoot, then pass those real paths into
isWithinDirectory (or update isWithinDirectory to perform the realpath
resolution internally) so containment is checked against resolved targets
(protecting against symlink escapes); also allow the root directory itself
(remove the relative.length > 0 rejection or treat an empty relative as inside)
so files that are exactly the root are permitted. Ensure you reference/modify
the functions isWithinDirectory and importClaudeSession and the variable
normalizedFilePath to implement this fix.
- Around line 253-319: The current loop builds per-message events and calls
fetch for each (see usedMessageIds, seqCounter,
sessionStateSchema.chunks.insert, event and the fetch to
`${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}/stream`), which is slow and
has no timeout or resume/cleanup on failure; change this by batching the events
into a single array (collect events instead of POSTing per iteration) and POST
the batch once to the stream endpoint (or a new batch endpoint), add an
AbortSignal timeout for the fetch (use AbortSignal.timeout(ms)) to avoid
hanging, and on fetch failure return structured partial-result info (e.g., count
of successfully created events and their messageIds) or call a cleanup path to
delete the partially-created session so callers can resume or handle partial
imports.
- Around line 67-88: The encodeClaudeProjectPath function currently strips
Windows drive letters causing mismatch with Claude's naming; update its
drive-handling so the colon is converted to a hyphen instead of removed (i.e.,
after normalizing path and replacing backslashes, replace a leading
drive-letter-colon sequence with drive-letter + hyphen), then continue the same
segment-sanitization and joining logic so Windows paths encode as Claude
expects; keep function name encodeClaudeProjectPath and the surrounding
normalize/segment sanitization steps intact.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 147-184: The Claude session list allows repeated imports because
DropdownMenuItem's onSelect always calls
importClaudeSessionMutation.mutateAsync; add a guard that checks whether the
claudeSession.filePath (or session title) is already imported before invoking
mutateAsync and instead show a toast or visually disable the item — e.g., derive
an importedFilePath set from existing sessions (or store import metadata), then
in the claudeSessions.map callback check
importedFilePaths.has(claudeSession.filePath) and if true either set
disabled={true} / add an "imported" indicator or early-return with toast; ensure
you still respect importClaudeSessionMutation.isPending, and update on
successful import to add the filePath to the imported set and call
onSelectSession as now.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 90-143: The file I/O loop in listClaudeSessions performs await
stat(filePath) sequentially (the withStats population), which is slow for many
files; change it to run stat calls in parallel using Promise.allSettled over
candidates.map(filePath => stat(filePath)) (or Promise.all with try/catch per
promise), then collect only fulfilled results and pair each resolved FileStat
with its filePath to build withStats, preserving the existing sort/slice/map
logic; reference listClaudeSessions, the candidates array and the withStats
population so you replace the sequential try/catch loop with a parallel
Promise.allSettled-based approach that filters out failures.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 148-177: The dropdown currently disables all Claude items using
importClaudeSessionMutation.isPending without indicating which specific item is
importing; add a per-item loading indicator and only disable the active item by
tracking the file being imported (e.g., introduce an importingSessionId state),
update the onSelect handler in the DropdownMenuItem for claudeSession.filePath
to set importingSessionId = claudeSession.filePath before calling
importClaudeSessionMutation.mutateAsync and clear it in finally, and render a
spinner or highlight inside the DropdownMenuItem when importingSessionId ===
claudeSession.filePath while setting disabled to
importClaudeSessionMutation.isPending && importingSessionId ===
claudeSession.filePath so other items remain interactive.
In `@packages/chat/src/shared/session-conversion.ts`:
- Around line 241-244: In both codexSessionConverter.convert and
claudeCodeSessionConverter.convert the call extractTimestamp(message ?? {}) is
evaluated twice; cache it in a local variable (e.g., timestampFromMessage =
extractTimestamp(message ?? {})) and then set createdAt using that variable: if
timestampFromMessage !== DEFAULT_TIMESTAMP use it, otherwise fall back to
extractTimestamp(record). Update both converters to use the cached
timestampFromMessage to avoid duplicate work and ensure the fallback still calls
extractTimestamp(record).
- Around line 255-278: claudeCodeSessionConverter.detect is too permissive — it
returns true for any record with message.role, which causes it to collide with
codex entries; tighten the detector in claudeCodeSessionConverter.detect (and
keep using helpers asRecord/asString/normalizeRole/extractText) by requiring
stronger discriminators (e.g., require that record.parentUuid/parent_uuid or
record.uuid exists and extractText(record).length > 0) and/or explicitly exclude
codex-style entries (check for kind === "codex_event" or presence of
codex-specific fields like turn_id/dir and return false). Ensure the updated
logic still allows legitimate Claude code sessions but prevents matching codex
entries.
- Around line 415-416: The module-level singleton
defaultSessionConverterRegistry (created with
createDefaultSessionConverterRegistry) is shared-mutable and can be mutated by
callers; change convertExternalSessionToChatChunks to instantiate a fresh
registry by calling createDefaultSessionConverterRegistry() inside the function
(instead of using the exported defaultSessionConverterRegistry) so each call
uses an independent registry, and remove or deprecate the mutable exported
singleton; update places that relied on the shared instance to accept a registry
parameter or rely on the per-call registry created in
convertExternalSessionToChatChunks.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/desktop/src/lib/trpc/routers/chat-service/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsxpackages/chat/src/shared/index.tspackages/chat/src/shared/session-conversion.test.tspackages/chat/src/shared/session-conversion.ts
Comment on lines
+67
to
+88
| function getClaudeProjectsRoot(): string { | ||
| return path.join(os.homedir(), ".claude", "projects"); | ||
| } | ||
|
|
||
| function encodeClaudeProjectPath(cwd: string): string { | ||
| const normalized = path.resolve(cwd).replace(/\\/g, "/"); | ||
| const withoutDrive = normalized.replace(/^[A-Za-z]:/, ""); | ||
| const segments = withoutDrive | ||
| .split("/") | ||
| .filter(Boolean) | ||
| .map((segment) => segment.replace(/[^A-Za-z0-9._-]/g, "-")); | ||
| return `-${segments.join("-")}`; | ||
| } | ||
|
|
||
| function isWithinDirectory(rootDir: string, targetPath: string): boolean { | ||
| const relative = path.relative(rootDir, targetPath); | ||
| return ( | ||
| relative.length > 0 && | ||
| !relative.startsWith("..") && | ||
| !path.isAbsolute(relative) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Claude Code CLI .claude/projects directory naming scheme
💡 Result:
Claude Code CLI stores per-project session history under ~/.claude/projects/ (or %USERPROFILE%\.claude\projects on Windows) in a directory name derived from the exact working directory path where you started claude. (linkedin.com)
Naming scheme (path → folder name)
It appears to be a simple path-encoding:
- Path separators become hyphens (
/or\→-) - On Unix/macOS, the leading
/becomes a leading- - On Windows, the drive + colon becomes
C-and then the separator becomes another-, producing e.g.C--Users-...(gist.github.com)
Examples:
/Users/name/Code/my-project→~/.claude/projects/-Users-name-Code-my-project/(gist.github.com)C:\Users\NAME\Documents\quickstuff\testenv→.claude\projects\C--Users-NAME-Documents-quickstuff-testenv\(gist.github.com)
Because it keys off the starting directory, claude --resume may not show sessions if you run it from a different path (or you moved/renamed the project folder). (linkedin.com)
Citations:
- 1: https://www.linkedin.com/posts/ian-paul-koskela_claudecode-devtips-buildinginpublic-activity-7361833254896746503-cdhN?utm_source=openai
- 2: https://gist.github.com/BoQsc/8b392c3293107edddbd00117ada0fdd2?utm_source=openai
- 3: https://gist.github.com/ericboehs/04b7c7f368e78bca3a3bdeaa2b8f2a70?utm_source=openai
- 4: https://gist.github.com/BoQsc/8b392c3293107edddbd00117ada0fdd2?utm_source=openai
- 5: https://www.linkedin.com/posts/ian-paul-koskela_claudecode-devtips-buildinginpublic-activity-7361833254896746503-cdhN?utm_source=openai
encodeClaudeProjectPath does not match Claude's actual directory naming scheme on Windows.
Claude preserves the drive letter and converts the colon to a hyphen (e.g., C:\Users\NAME\... → C--Users-NAME-...), but the current implementation strips the drive letter entirely. This causes workspace-scoped listing to always miss sessions on Windows and silently fall back to all sessions.
Fix: Replace the drive letter removal logic to convert the colon to a hyphen instead:
const withoutDrive = normalized.replace(/^([A-Za-z]):/, "$1-");
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 67 -
88, The encodeClaudeProjectPath function currently strips Windows drive letters
causing mismatch with Claude's naming; update its drive-handling so the colon is
converted to a hyphen instead of removed (i.e., after normalizing path and
replacing backslashes, replace a leading drive-letter-colon sequence with
drive-letter + hyphen), then continue the same segment-sanitization and joining
logic so Windows paths encode as Claude expects; keep function name
encodeClaudeProjectPath and the surrounding normalize/segment sanitization steps
intact.
Comment on lines
+81
to
+88
| function isWithinDirectory(rootDir: string, targetPath: string): boolean { | ||
| const relative = path.relative(rootDir, targetPath); | ||
| return ( | ||
| relative.length > 0 && | ||
| !relative.startsWith("..") && | ||
| !path.isAbsolute(relative) | ||
| ); | ||
| } |
Contributor
There was a problem hiding this comment.
isWithinDirectory rejects the root directory itself (relative.length > 0), which is correct, but does not guard against symlink escapes.
path.relative computes a textual relative path — if the target is a symlink pointing outside claudeProjectsRoot, the check passes but the resolved file is outside the intended boundary. Since followSymbolicLinks: false is set in fast-glob (line 103), listed paths won't follow symlinks, but importClaudeSession receives filePath from user input directly and calls path.resolve + readFile on it.
Consider using fs.realpath on normalizedFilePath before the containment check to guard against symlink-based traversal.
🔒 Proposed fix
+import { realpath } from "node:fs/promises";
+
async function importClaudeSession(args: {
filePath: string;
organizationId: string;
workspaceId: string;
}): Promise<{...}> {
const claudeProjectsRoot = getClaudeProjectsRoot();
- const normalizedFilePath = path.resolve(args.filePath);
+ const normalizedFilePath = await realpath(path.resolve(args.filePath));
+ const resolvedRoot = await realpath(claudeProjectsRoot);
if (!existsSync(claudeProjectsRoot)) {
...
}
- if (!isWithinDirectory(claudeProjectsRoot, normalizedFilePath)) {
+ if (!isWithinDirectory(resolvedRoot, normalizedFilePath)) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 81 -
88, The isWithinDirectory check should resolve symlinks before comparing paths:
in importClaudeSession, call fs.realpath (or fs.realpathSync) on
normalizedFilePath and on claudeProjectsRoot, then pass those real paths into
isWithinDirectory (or update isWithinDirectory to perform the realpath
resolution internally) so containment is checked against resolved targets
(protecting against symlink escapes); also allow the root directory itself
(remove the relative.length > 0 rejection or treat an empty relative as inside)
so files that are exactly the root are permitted. Ensure you reference/modify
the functions isWithinDirectory and importClaudeSession and the variable
normalizedFilePath to implement this fix.
Comment on lines
+108
to
+116
| const workspaceMatches = filePaths.filter((filePath) => { | ||
| const relative = path | ||
| .relative(claudeProjectsRoot, filePath) | ||
| .replace(/\\/g, "/"); | ||
| const [projectId] = relative.split("/"); | ||
| return projectId === workspaceProjectId; | ||
| }); | ||
|
|
||
| const candidates = workspaceMatches.length > 0 ? workspaceMatches : filePaths; |
Contributor
There was a problem hiding this comment.
Fallback to all Claude sessions when no workspace-specific match is found may leak unrelated project data.
Line 116: when workspaceMatches is empty, candidates falls back to the entire filePaths array — every .jsonl across all Claude projects. This means a user could see (and import) sessions from a completely unrelated project. If this is intentional (discoverability), consider at least labeling them differently in the UI. If not, return an empty list instead.
🛡️ Restrict to workspace-only sessions
- const candidates = workspaceMatches.length > 0 ? workspaceMatches : filePaths;
+ const candidates = workspaceMatches;📝 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.
Suggested change
| const workspaceMatches = filePaths.filter((filePath) => { | |
| const relative = path | |
| .relative(claudeProjectsRoot, filePath) | |
| .replace(/\\/g, "/"); | |
| const [projectId] = relative.split("/"); | |
| return projectId === workspaceProjectId; | |
| }); | |
| const candidates = workspaceMatches.length > 0 ? workspaceMatches : filePaths; | |
| const workspaceMatches = filePaths.filter((filePath) => { | |
| const relative = path | |
| .relative(claudeProjectsRoot, filePath) | |
| .replace(/\\/g, "/"); | |
| const [projectId] = relative.split("/"); | |
| return projectId === workspaceProjectId; | |
| }); | |
| const candidates = workspaceMatches; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 108 -
116, The current logic sets candidates = workspaceMatches.length > 0 ?
workspaceMatches : filePaths which falls back to all Claude sessions and can
leak other projects; change it so that when workspaceMatches is empty you return
an empty array (or otherwise restrict to workspace-only sessions) instead of
using filePaths. Update the code using the symbols workspaceMatches, candidates,
workspaceProjectId and filePaths so candidates = workspaceMatches (or [] when no
matches) and ensure the caller/return path respects that empty list result so
unrelated .jsonl sessions are not exposed.
Comment on lines
+253
to
+319
| const usedMessageIds = new Set<string>(); | ||
| let seqCounter = 0; | ||
| for (const [index, chunk] of converted.messages.entries()) { | ||
| const baseMessageId = | ||
| typeof chunk.message.id === "string" && chunk.message.id.trim().length > 0 | ||
| ? chunk.message.id.trim() | ||
| : `imported-${index}`; | ||
| let messageId = baseMessageId; | ||
| let duplicateCounter = 1; | ||
| while (usedMessageIds.has(messageId)) { | ||
| messageId = `${baseMessageId}-${duplicateCounter++}`; | ||
| } | ||
| usedMessageIds.add(messageId); | ||
|
|
||
| const createdAtRaw = chunk.message.createdAt; | ||
| const createdAt = | ||
| typeof createdAtRaw === "string" | ||
| ? createdAtRaw | ||
| : createdAtRaw instanceof Date | ||
| ? createdAtRaw.toISOString() | ||
| : new Date().toISOString(); | ||
| const role = chunk.message.role; | ||
| const actorId = | ||
| role === "user" | ||
| ? "imported-claude-user" | ||
| : role === "assistant" | ||
| ? "imported-claude-assistant" | ||
| : "imported-claude-system"; | ||
|
|
||
| const event = sessionStateSchema.chunks.insert({ | ||
| key: `${messageId}:0`, | ||
| value: { | ||
| messageId, | ||
| actorId, | ||
| role, | ||
| chunk: JSON.stringify({ | ||
| type: "whole-message", | ||
| message: { | ||
| ...chunk.message, | ||
| id: messageId, | ||
| createdAt, | ||
| }, | ||
| }), | ||
| seq: seqCounter++, | ||
| createdAt, | ||
| }, | ||
| }); | ||
|
|
||
| const appendResponse = await fetch( | ||
| `${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}/stream`, | ||
| { | ||
| method: "POST", | ||
| headers: { | ||
| ...headers, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify(event), | ||
| }, | ||
| ); | ||
| if (!appendResponse.ok) { | ||
| const detail = await appendResponse.text().catch(() => ""); | ||
| throw new TRPCError({ | ||
| code: "INTERNAL_SERVER_ERROR", | ||
| message: `Failed to append imported message (${appendResponse.status}): ${detail || "unknown error"}`, | ||
| }); | ||
| } | ||
| } |
Contributor
There was a problem hiding this comment.
Sequential per-message HTTP POST with no timeout — slow imports and no partial-failure recovery.
Each converted message is appended via a separate fetch POST (line 301-311). For sessions with many messages this will be slow and latency-bound. Additionally:
- No
AbortSignal/ timeout on any of thefetchcalls in this file — a hung API will block the tRPC mutation indefinitely and tie up the Electron main process. - Partial import on failure: if an append fails at message N, the session already exists with messages 0..N-1 but the mutation throws. The caller gets an error with no way to resume or clean up the partially imported session.
Consider batching messages (if the API supports it), adding a timeout via AbortSignal.timeout(ms), and either cleaning up on failure or returning partial-success info so the caller can handle it.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 253 -
319, The current loop builds per-message events and calls fetch for each (see
usedMessageIds, seqCounter, sessionStateSchema.chunks.insert, event and the
fetch to `${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}/stream`), which is
slow and has no timeout or resume/cleanup on failure; change this by batching
the events into a single array (collect events instead of POSTing per iteration)
and POST the batch once to the stream endpoint (or a new batch endpoint), add an
AbortSignal timeout for the fetch (use AbortSignal.timeout(ms)) to avoid
hanging, and on fetch failure return structured partial-result info (e.g., count
of successfully created events and their messageIds) or call a cleanup path to
delete the partially-created session so callers can resume or handle partial
imports.
Comment on lines
+370
to
+400
| export const createChatServiceClaudeRouter = () => | ||
| router({ | ||
| listSessions: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| cwd: z.string().min(1), | ||
| limit: z.number().int().min(1).max(200).default(30), | ||
| }), | ||
| ) | ||
| .query(async ({ input }) => { | ||
| return listClaudeSessions({ | ||
| cwd: input.cwd, | ||
| limit: input.limit, | ||
| }); | ||
| }), | ||
| importSession: publicProcedure | ||
| .input( | ||
| z.object({ | ||
| filePath: z.string().min(1), | ||
| organizationId: z.string().min(1), | ||
| workspaceId: z.string().min(1), | ||
| }), | ||
| ) | ||
| .mutation(async ({ input }) => { | ||
| return importClaudeSession({ | ||
| filePath: input.filePath, | ||
| organizationId: input.organizationId, | ||
| workspaceId: input.workspaceId, | ||
| }); | ||
| }), | ||
| }); |
Contributor
There was a problem hiding this comment.
listSessions uses publicProcedure — no auth check.
importSession validates auth inside importClaudeSession via getAuthHeaders(), but listSessions has no authentication. This allows any renderer context (or potentially a malicious script via IPC) to enumerate Claude session files on disk without being signed in. If this is intentional for UX, fine — just flagging for awareness.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 370 -
400, The listSessions endpoint is using publicProcedure (function name:
listSessions) which lacks authentication and can expose local Claude session
files via listClaudeSessions; change it to require auth the same way
importSession does (see importSession -> importClaudeSession which uses
getAuthHeaders) by switching listSessions to the protected/authenticated
procedure wrapper or explicitly validate auth (call the same getAuthHeaders or
auth-check helper before invoking listClaudeSessions) so only authenticated
users can enumerate sessions.
Comment on lines
+147
to
+184
| claudeSessions.map((claudeSession) => ( | ||
| <DropdownMenuItem | ||
| key={claudeSession.filePath} | ||
| disabled={importClaudeSessionMutation.isPending} | ||
| className="group flex items-center justify-between gap-2" | ||
| onSelect={(event) => { | ||
| event.preventDefault(); | ||
| if (!organizationId) { | ||
| toast.error("Organization is required to import session"); | ||
| return; | ||
| } | ||
| void (async () => { | ||
| try { | ||
| const imported = | ||
| await importClaudeSessionMutation.mutateAsync({ | ||
| filePath: claudeSession.filePath, | ||
| organizationId, | ||
| workspaceId, | ||
| }); | ||
| onSelectSession(imported.sessionId); | ||
| setIsOpen(false); | ||
| toast.success("Claude session imported"); | ||
| } catch (error) { | ||
| toast.error( | ||
| error instanceof Error | ||
| ? error.message | ||
| : "Failed to import Claude session", | ||
| ); | ||
| } | ||
| })(); | ||
| }} | ||
| > | ||
| <span className="min-w-0 truncate text-xs"> | ||
| {claudeSession.title} | ||
| </span> | ||
| <HiMiniArrowDownTray className="size-3 shrink-0 text-muted-foreground group-hover:text-foreground" /> | ||
| </DropdownMenuItem> | ||
| )) |
Contributor
There was a problem hiding this comment.
No guard against re-importing the same Claude session.
A user can click the same Claude session entry multiple times (across different dropdown opens), creating duplicate imported sessions each time. Consider tracking already-imported file paths (e.g., by checking existing session titles or storing import metadata) or at minimum showing a visual indicator for sessions that have already been imported.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`
around lines 147 - 184, The Claude session list allows repeated imports because
DropdownMenuItem's onSelect always calls
importClaudeSessionMutation.mutateAsync; add a guard that checks whether the
claudeSession.filePath (or session title) is already imported before invoking
mutateAsync and instead show a toast or visually disable the item — e.g., derive
an importedFilePath set from existing sessions (or store import metadata), then
in the claudeSessions.map callback check
importedFilePaths.has(claudeSession.filePath) and if true either set
disabled={true} / add an "imported" indicator or early-return with toast; ensure
you still respect importClaudeSessionMutation.isPending, and update on
successful import to add the filePath to the imported set and call
onSelectSession as now.
Contributor
There was a problem hiding this comment.
♻️ Duplicate comments (5)
apps/desktop/src/lib/trpc/routers/chat-service/index.ts (4)
372-384:⚠️ Potential issue | 🟠 Major
listSessionsremains unauthenticated while exposing local filesystem metadata.Line 372 uses
publicProcedure, so Claude session file enumeration can happen without the auth gate used by import.🔧 Proposed fix
listSessions: publicProcedure @@ .query(async ({ input }) => { + await getAuthHeaders(); return listClaudeSessions({ cwd: input.cwd, limit: input.limit, }); }),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 372 - 384, The listSessions trpc handler is declared with publicProcedure and exposes local filesystem session metadata; change its declaration to use the authenticated/protected procedure used elsewhere (e.g., protectedProcedure or authenticatedProcedure) so calls to listSessions require a logged-in user, and keep calling listClaudeSessions({ cwd: input.cwd, limit: input.limit }) unchanged; update any imports/usage to match the project's auth wrapper and add a brief test that unauthenticated requests to listSessions are rejected.
231-319:⚠️ Potential issue | 🟠 MajorImport flow still risks indefinite hangs and partial session writes.
Line 231 and Line 301 perform network calls without timeout, and append failure mid-loop leaves partially imported sessions without cleanup/resume signaling.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 231 - 319, The import loop uses fetch calls (createResponse and appendResponse) with no timeouts and no rollback/cleanup on partial failure, risking hangs and partial session writes; update the import flow in the function handling session import to use abortable fetch requests (AbortController with a sensible timeout) for the initial PUT to /api/chat/${sessionId} and each POST to /api/chat/${sessionId}/stream, and on any non-ok response or fetch error ensure you signal/clean up the half-written session (e.g., call a sessionState cleanup or a DELETE/finalize endpoint and/or insert a failure marker via sessionStateSchema.chunks.insert with the sessionId/seqCounter) so partial imports are either rolled back or marked resumable; reference createResponse, appendResponse, sessionStateSchema.chunks.insert, sessionId and seqCounter when making the changes.
81-88:⚠️ Potential issue | 🟠 MajorPath containment check is still bypassable via symlink targets.
Line 82 compares lexical paths only. In
importClaudeSession, a symlink inside.claude/projectscan resolve outside root and still pass this check beforereadFile.🔒 Proposed fix
-import { readFile, stat } from "node:fs/promises"; +import { readFile, realpath, stat } from "node:fs/promises"; @@ const claudeProjectsRoot = getClaudeProjectsRoot(); - const normalizedFilePath = path.resolve(args.filePath); + const requestedFilePath = path.resolve(args.filePath); @@ - if (!isWithinDirectory(claudeProjectsRoot, normalizedFilePath)) { + const [resolvedRootPath, resolvedFilePath] = await Promise.all([ + realpath(claudeProjectsRoot), + realpath(requestedFilePath), + ]); + if (!isWithinDirectory(resolvedRootPath, resolvedFilePath)) { throw new TRPCError({ code: "BAD_REQUEST", message: "Invalid Claude session path", }); } - if (!normalizedFilePath.endsWith(".jsonl")) { + if (!resolvedFilePath.endsWith(".jsonl")) { throw new TRPCError({ code: "BAD_REQUEST", message: "Claude session file must be .jsonl", }); } @@ - const sessionFileContent = await readFile(normalizedFilePath, "utf8"); + const sessionFileContent = await readFile(resolvedFilePath, "utf8");Also applies to: 196-205
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 81 - 88, The current isWithinDirectory(rootDir, targetPath) does a lexical check that can be bypassed by symlinks; update it to resolve symlinks before comparing by calling fs.promises.realpath (or fs.realpathSync) on both rootDir and targetPath (handle errors by returning false), then compute path.relative on the resolved paths and apply the same checks; also apply the same realpath-based fix in the importClaudeSession code path (the code that reads files under .claude/projects) so any symlink targets are resolved prior to calling readFile.
71-79:⚠️ Potential issue | 🟠 MajorWindows project-path encoding still breaks workspace scoping and falls back to global listing.
Line 73 removes the drive prefix and Line 78 always prepends
-, so Windows paths won’t map to Claude’s project folder naming. Then Line 116 falls back to all projects, exposing unrelated sessions in the dropdown.🔧 Proposed fix
function encodeClaudeProjectPath(cwd: string): string { const normalized = path.resolve(cwd).replace(/\\/g, "/"); - const withoutDrive = normalized.replace(/^[A-Za-z]:/, ""); - const segments = withoutDrive + const encodedDrive = normalized.replace(/^([A-Za-z]):/, "$1-"); + const segments = encodedDrive .split("/") .filter(Boolean) .map((segment) => segment.replace(/[^A-Za-z0-9_-]/g, "-")); - return `-${segments.join("-")}`; + const prefix = normalized.startsWith("/") ? "-" : ""; + return `${prefix}${segments.join("-")}`; } @@ - const candidates = workspaceMatches.length > 0 ? workspaceMatches : filePaths; + const candidates = workspaceMatches;Also applies to: 116-116
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 71 - 79, The Windows path encoding drops the drive letter and always prepends a hyphen which mis-maps Windows workspaces to Claude project names; update encodeClaudeProjectPath to preserve the drive as the first segment (e.g., convert "C:" to "C" or "c" and include it in the segments array instead of stripping it) and stop unconditionally adding a leading "-" so the returned value is segments.join("-") (after the existing sanitize map) — this ensures Windows paths produce the same stable project-token format as POSIX paths and prevents the fallback to global listing.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx (1)
155-185:⚠️ Potential issue | 🟡 MinorSame Claude file can still be imported repeatedly, creating duplicate sessions.
Line 160 always invokes import, so the same Claude entry can be re-imported multiple times. Add an “already imported” guard/disable path at least in-memory for the current UI session.
🔧 Minimal guard example
+ const [importedClaudePaths, setImportedClaudePaths] = useState<Set<string>>( + () => new Set(), + ); @@ - disabled={importClaudeSessionMutation.isPending} + disabled={ + importClaudeSessionMutation.isPending || + importedClaudePaths.has(claudeSession.filePath) + } @@ const imported = await importClaudeSessionMutation.mutateAsync({ filePath: claudeSession.filePath, organizationId, workspaceId, }); + setImportedClaudePaths((prev) => { + const next = new Set(prev); + next.add(claudeSession.filePath); + return next; + });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx` around lines 155 - 185, Prevent duplicate imports by adding an in-memory guard that tracks already-imported Claude filePaths and skips/disables import when seen: create a local state (e.g., importedClaudeFilePaths: Set<string>) in the component and use it to disable the DropdownMenuItem or early-return in the onSelect handler for a given claudeSession.filePath before calling importClaudeSessionMutation.mutateAsync; on successful import (after importClaudeSessionMutation resolves) add the filePath to the Set, call onSelectSession(imported.sessionId) and setIsOpen(false) as before, and keep importClaudeSessionMutation.isPending logic to avoid concurrent duplicates.
🧹 Nitpick comments (1)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
62-77: Claude import query/mutation UI is duplicated in two SessionSelector components.This block is effectively mirrored from
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx, which will drift over time. Consider extracting a shared hook/component for Claude session listing/import behavior.Also applies to: 144-201
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 62 - 77, The Claude session listing and import logic is duplicated; extract a shared hook (e.g., useClaudeSessions) that wraps electronTrpc.chatServiceClaude.listSessions.useQuery and electronTrpc.chatServiceClaude.importSession.useMutation and accepts parameters like cwd and isOpen; replace the duplicated blocks in both SessionSelector components with calls to this new hook (keep same option keys: enabled, staleTime, and the cwd/limit payload) and surface the returned values (claudeSessions, isLoadingClaudeSessions, claudeSessionsError, importClaudeSessionMutation) so the UI code remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 372-384: The listSessions trpc handler is declared with
publicProcedure and exposes local filesystem session metadata; change its
declaration to use the authenticated/protected procedure used elsewhere (e.g.,
protectedProcedure or authenticatedProcedure) so calls to listSessions require a
logged-in user, and keep calling listClaudeSessions({ cwd: input.cwd, limit:
input.limit }) unchanged; update any imports/usage to match the project's auth
wrapper and add a brief test that unauthenticated requests to listSessions are
rejected.
- Around line 231-319: The import loop uses fetch calls (createResponse and
appendResponse) with no timeouts and no rollback/cleanup on partial failure,
risking hangs and partial session writes; update the import flow in the function
handling session import to use abortable fetch requests (AbortController with a
sensible timeout) for the initial PUT to /api/chat/${sessionId} and each POST to
/api/chat/${sessionId}/stream, and on any non-ok response or fetch error ensure
you signal/clean up the half-written session (e.g., call a sessionState cleanup
or a DELETE/finalize endpoint and/or insert a failure marker via
sessionStateSchema.chunks.insert with the sessionId/seqCounter) so partial
imports are either rolled back or marked resumable; reference createResponse,
appendResponse, sessionStateSchema.chunks.insert, sessionId and seqCounter when
making the changes.
- Around line 81-88: The current isWithinDirectory(rootDir, targetPath) does a
lexical check that can be bypassed by symlinks; update it to resolve symlinks
before comparing by calling fs.promises.realpath (or fs.realpathSync) on both
rootDir and targetPath (handle errors by returning false), then compute
path.relative on the resolved paths and apply the same checks; also apply the
same realpath-based fix in the importClaudeSession code path (the code that
reads files under .claude/projects) so any symlink targets are resolved prior to
calling readFile.
- Around line 71-79: The Windows path encoding drops the drive letter and always
prepends a hyphen which mis-maps Windows workspaces to Claude project names;
update encodeClaudeProjectPath to preserve the drive as the first segment (e.g.,
convert "C:" to "C" or "c" and include it in the segments array instead of
stripping it) and stop unconditionally adding a leading "-" so the returned
value is segments.join("-") (after the existing sanitize map) — this ensures
Windows paths produce the same stable project-token format as POSIX paths and
prevents the fallback to global listing.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 155-185: Prevent duplicate imports by adding an in-memory guard
that tracks already-imported Claude filePaths and skips/disables import when
seen: create a local state (e.g., importedClaudeFilePaths: Set<string>) in the
component and use it to disable the DropdownMenuItem or early-return in the
onSelect handler for a given claudeSession.filePath before calling
importClaudeSessionMutation.mutateAsync; on successful import (after
importClaudeSessionMutation resolves) add the filePath to the Set, call
onSelectSession(imported.sessionId) and setIsOpen(false) as before, and keep
importClaudeSessionMutation.isPending logic to avoid concurrent duplicates.
---
Nitpick comments:
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 62-77: The Claude session listing and import logic is duplicated;
extract a shared hook (e.g., useClaudeSessions) that wraps
electronTrpc.chatServiceClaude.listSessions.useQuery and
electronTrpc.chatServiceClaude.importSession.useMutation and accepts parameters
like cwd and isOpen; replace the duplicated blocks in both SessionSelector
components with calls to this new hook (keep same option keys: enabled,
staleTime, and the cwd/limit payload) and surface the returned values
(claudeSessions, isLoadingClaudeSessions, claudeSessionsError,
importClaudeSessionMutation) so the UI code remains unchanged.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/desktop/src/lib/trpc/routers/chat-service/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
♻️ Duplicate comments (6)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx (1)
173-213:⚠️ Potential issue | 🟡 MinorGuard against re-importing the same Claude session file.
Line 178 still imports unconditionally, so the same
claudeSession.filePathcan be imported repeatedly across menu opens. Add an already-imported guard (disable or early return + toast) keyed by file path.Suggested patch
+ const [importedClaudeFilePaths, setImportedClaudeFilePaths] = useState<Set<string>>(new Set()); ... {claudeSessions.map((claudeSession) => ( + const isAlreadyImported = importedClaudeFilePaths.has(claudeSession.filePath); <DropdownMenuItem key={claudeSession.filePath} - disabled={importClaudeSessionMutation.isPending} + disabled={importClaudeSessionMutation.isPending || isAlreadyImported} className="group flex items-center justify-between gap-2" onSelect={(event) => { event.preventDefault(); + if (isAlreadyImported) { + toast.info("Claude session already imported"); + return; + } if (!organizationId) { toast.error("Organization is required to import session"); return; } void (async () => { try { const imported = await importClaudeSessionMutation.mutateAsync({ filePath: claudeSession.filePath, organizationId, workspaceId, }); + setImportedClaudeFilePaths((prev) => new Set(prev).add(claudeSession.filePath)); onSelectSession(imported.sessionId); setIsOpen(false); toast.success("Claude session imported");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx` around lines 173 - 213, The menu currently allows re-importing the same Claude session because the onSelect handler in the DropdownMenuItem (inside the claudeSessions.map) always calls importClaudeSessionMutation; add a guard that checks whether claudeSession.filePath is already imported (use whatever source of truth exists: e.g., an importedSessions list, workspace sessions, or a new Set of imported file paths) and if so either disable the DropdownMenuItem (set disabled when filePath is present) or early-return in the onSelect handler with toast.error("Session already imported") to prevent duplicate imports; update the DropdownMenuItem props and the onSelect logic around importClaudeSessionMutation.mutateAsync, and keep calls to onSelectSession and setIsOpen only when a new import actually occurs.apps/desktop/src/lib/trpc/routers/chat-service/index.ts (5)
97-104:isWithinDirectoryis still vulnerable to symlink-based path traversal.
path.relativeoperates on textual paths; a symlink insideclaudeProjectsRootpointing outside it would pass the containment check.importClaudeSessionresolves and reads the file directly from user-supplied input, so this is an active attack surface.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 97 - 104, The isWithinDirectory function is vulnerable to symlink traversal; update it to resolve real filesystem paths before comparison: call fs.realpath (or fs.realpathSync) on both rootDir and targetPath (after path.resolve), normalize and ensure the resolved target path begins with the resolved root path + path.sep (to avoid partial-match attacks). Apply this change where importClaudeSession uses isWithinDirectory so the containment check verifies actual filesystem locations rather than raw textual paths.
411-441:listSessionsonpublicProcedure— still unauthenticated.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 411 - 441, The router factory createChatServiceClaudeRouter exposes listSessions and importSession using publicProcedure (unauthenticated); change them to the authenticated procedure used in your app (e.g., protectedProcedure or the project’s authProcedure) so only logged-in users can call listClaudeSessions and importClaudeSession; update the two procedure declarations (listSessions and importSession) to use the auth wrapper or add the auth middleware used elsewhere in your routers to enforce organization/workspace ownership before calling listClaudeSessions and importClaudeSession.
87-95:encodeClaudeProjectPathstill strips the Windows drive letter instead of converting the colon to a hyphen.Claude stores projects under e.g.
C--Users-NAME-...on Windows (colon becomes a hyphen, drive letter is kept), but line 89 strips the entire drive letter, causing workspace-scoped listing to silently miss all sessions on Windows and fall back to showing all sessions.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 87 - 95, The function encodeClaudeProjectPath currently removes the Windows drive letter with withoutDrive = normalized.replace(/^[A-Za-z]:/, "") which drops the drive instead of converting the colon to a hyphen; update that step so the drive letter is preserved and the colon is replaced (e.g. normalized.replace(/^([A-Za-z]):/, "$1-") or simply normalized.replace(":", "-")) before splitting into segments (refer to the variables normalized and the function encodeClaudeProjectPath) so Windows paths become e.g. C--Users-NAME-... instead of losing the drive.
156-156: Fallback to all-sessions still leaks unrelated project data.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` at line 156, The current fallback sets candidates = workspaceMatches.length > 0 ? workspaceMatches : allEntries which leaks unrelated project/session data; instead restrict the fallback to only entries belonging to the current workspace/project (e.g., filter allEntries by entry.workspaceId or entry.projectId) or return an empty list; modify the assignment so candidates is workspaceMatches when present, otherwise allEntries.filter(e => e.workspaceId === currentWorkspaceId || e.projectId === currentProjectId) (or [] if no workspace context) and ensure you reference the same workspace identifier used elsewhere in this module.
294-360: Sequential per-messagefetchwith noAbortSignaltimeout and no partial-failure cleanup.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 294 - 360, The loop sends each appended event via sequential fetch without timeouts or rollback; update the logic around sessionStateSchema.chunks.insert and the POST to `${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}/stream` so that you: 1) create an AbortController per request with a reasonable timeout (use setTimeout to call controller.abort) and pass controller.signal to fetch; 2) run the per-message uploads with controlled parallelism (e.g., collect promises and use Promise.allSettled or a small concurrency pool) instead of strictly sequential await to improve throughput; and 3) if any appendResponse is non-ok or any promise rejects, cancel outstanding requests, and perform a cleanup pass that removes previously inserted chunks (use the same keys created in sessionStateSchema.chunks.insert, e.g., `${messageId}:0`) before throwing a TRPCError, ensuring you still include the appendResponse.status/detail in the thrown error.
🧹 Nitpick comments (2)
apps/desktop/src/lib/trpc/routers/chat-service/index.ts (1)
119-168: Sequentialstat()calls in a hot loop — parallelise for better performance.Lines 163–168 stat each candidate file one at a time. With many JSONL files this adds unnecessary latency on every dropdown open. Use
Promise.allSettledto fan-out concurrently.♻️ Proposed refactor
- const withStats: Array<{ - filePath: string; - fileStat: FileStat; - root: ClaudeSessionRoot; - }> = []; - for (const { filePath, root } of candidates) { - try { - const fileStat = await stat(filePath); - withStats.push({ filePath, fileStat, root }); - } catch {} - } + const statResults = await Promise.allSettled( + candidates.map(({ filePath, root }) => + stat(filePath).then((fileStat) => ({ filePath, fileStat, root })), + ), + ); + const withStats = statResults + .filter( + (r): r is PromiseFulfilledResult<{ filePath: string; fileStat: FileStat; root: ClaudeSessionRoot }> => + r.status === "fulfilled", + ) + .map((r) => r.value);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 119 - 168, In listClaudeSessions, the loop that calls stat(filePath) sequentially (iterating over candidates and pushing to withStats) creates latency; replace it with a concurrent fan-out using Promise.allSettled over candidates.map(({filePath, root}) => stat(filePath).then(fileStat => ({filePath, fileStat, root}))). After Promise.allSettled, filter fulfilled results to populate withStats and ignore rejected entries (or log them), preserving the same shape used later; ensure you still return early when withStats is empty and keep references to candidates, filePath, root, fileStat unchanged.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
88-90:requiresMainRestartrelies on fragile tRPC internal error-message substrings.Matching on
"No \"query\"-procedure"and"No procedure"couples the UX hint to tRPC's private error message format, which can change across tRPC versions or be triggered by unrelated errors. Consider using a tRPC error code check or a dedicated sentinel value instead.♻️ Proposed refactor
- const requiresMainRestart = - claudeSessionsErrorMessage.includes("No \"query\"-procedure") || - claudeSessionsErrorMessage.includes("No procedure"); + // TRPCClientError exposes a `data.code` field; use that when available. + const requiresMainRestart = + claudeSessionsError != null && + typeof (claudeSessionsError as { data?: { code?: string } }).data?.code === + "string" && + (claudeSessionsError as { data?: { code?: string } }).data?.code === + "NOT_FOUND";Alternatively, expose a typed error shape from the
listSessionsprocedure so the renderer can distinguish "route not registered" from other failures without string matching.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 88 - 90, The current requiresMainRestart boolean (based on claudeSessionsErrorMessage substring checks) is fragile; change the code to avoid string-matching by surfacing a typed error or sentinel from the listSessions call and then checking that structured value (e.g., an error.code or error.type such as "ROUTE_NOT_REGISTERED") instead of matching "No \"query\"-procedure"/"No procedure"; update the listSessions procedure to return a clear error shape or sentinel and then update the requiresMainRestart logic (which currently references requiresMainRestart and claudeSessionsErrorMessage) to inspect that structured error (or tRPC error.code) so the renderer can reliably detect the "route not registered" case.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 122-126: The CI formatter failure means this file's
whitespace/formatting doesn't match project style; run the project's formatter
(bunx biome check --write) on
apps/desktop/src/lib/trpc/routers/chat-service/index.ts or manually reformat the
block around the function that returns Promise<ClaudeSessionSummary[]> (the
lines using const { cwd, limit } = args; const roots =
getClaudeSessionRoots().filter((root) => existsSync(root.rootDir)); if
(roots.length === 0) return [];), ensuring spacing and line breaks conform to
biome rules so the CI check passes.
- Around line 258-268: Wrap the call to convertExternalSessionToChatChunks in a
try/catch so malformed or unexpected .jsonl content produces a descriptive
TRPCError instead of bubbling a raw exception; catch errors around
convertExternalSessionToChatChunks (while keeping readFile as-is), and on
failure throw new TRPCError({ code: "BAD_REQUEST", message: `Invalid Claude
session file: ${error.message}` }) (optionally log the error) before proceeding
to the existing check for converted.messages.length === 0.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 72-81: The query currently falls back to cwd = "/" which causes
encodeClaudeProjectPath("/") -> "-" and makes workspaceMatches empty so
listClaudeSessions returns allEntries; to fix, ensure the caller never passes an
empty cwd or fail-fast here: in the
electronTrpc.chatServiceClaude.listSessions.useQuery call (and related
session-selection flow) assert cwd is non-empty (e.g., return/throw or set
enabled: false when cwd.trim().length === 0) or replace the "/" fallback with a
clear sentinel (e.g., undefined) and/or add a comment documenting why "/" was
chosen and the security implication; reference encodeClaudeProjectPath,
workspaceMatches, listClaudeSessions and allEntries when making the change so
the behaviour is explicit.
- Around line 87-90: The CI formatter failed due to string quoting/line-break
style around the claudeSessionsErrorMessage assignment and its use in
requiresMainRestart; run the project's formatter (bunx biome check --write) on
this file or manually normalize the string quotes and line breaks so the ternary
and the subsequent boolean checks (claudeSessionsErrorMessage and
requiresMainRestart) conform to the repo style, ensuring consistent use of
double/single quotes and keeping each logical expression on its own line.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 65-79: Reformat the
electronTrpc.chatServiceClaude.listSessions.useQuery call block to satisfy the
project's formatter (biome/prettier): adjust indentation and line breaks in the
destructuring and the two argument objects so the call is formatted consistently
(keep the same props: { cwd: cwd.trim().length > 0 ? cwd : "/", limit: 20 } and
options { enabled: isOpen, staleTime: 30_000 }), ensuring the const destructure
lines for data/isLoading/error (claudeSessions, isLoadingClaudeSessions,
claudeSessionsError) and the useQuery invocation match the project's style rules
before committing.
---
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 97-104: The isWithinDirectory function is vulnerable to symlink
traversal; update it to resolve real filesystem paths before comparison: call
fs.realpath (or fs.realpathSync) on both rootDir and targetPath (after
path.resolve), normalize and ensure the resolved target path begins with the
resolved root path + path.sep (to avoid partial-match attacks). Apply this
change where importClaudeSession uses isWithinDirectory so the containment check
verifies actual filesystem locations rather than raw textual paths.
- Around line 411-441: The router factory createChatServiceClaudeRouter exposes
listSessions and importSession using publicProcedure (unauthenticated); change
them to the authenticated procedure used in your app (e.g., protectedProcedure
or the project’s authProcedure) so only logged-in users can call
listClaudeSessions and importClaudeSession; update the two procedure
declarations (listSessions and importSession) to use the auth wrapper or add the
auth middleware used elsewhere in your routers to enforce organization/workspace
ownership before calling listClaudeSessions and importClaudeSession.
- Around line 87-95: The function encodeClaudeProjectPath currently removes the
Windows drive letter with withoutDrive = normalized.replace(/^[A-Za-z]:/, "")
which drops the drive instead of converting the colon to a hyphen; update that
step so the drive letter is preserved and the colon is replaced (e.g.
normalized.replace(/^([A-Za-z]):/, "$1-") or simply normalized.replace(":",
"-")) before splitting into segments (refer to the variables normalized and the
function encodeClaudeProjectPath) so Windows paths become e.g. C--Users-NAME-...
instead of losing the drive.
- Line 156: The current fallback sets candidates = workspaceMatches.length > 0 ?
workspaceMatches : allEntries which leaks unrelated project/session data;
instead restrict the fallback to only entries belonging to the current
workspace/project (e.g., filter allEntries by entry.workspaceId or
entry.projectId) or return an empty list; modify the assignment so candidates is
workspaceMatches when present, otherwise allEntries.filter(e => e.workspaceId
=== currentWorkspaceId || e.projectId === currentProjectId) (or [] if no
workspace context) and ensure you reference the same workspace identifier used
elsewhere in this module.
- Around line 294-360: The loop sends each appended event via sequential fetch
without timeouts or rollback; update the logic around
sessionStateSchema.chunks.insert and the POST to
`${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}/stream` so that you: 1) create
an AbortController per request with a reasonable timeout (use setTimeout to call
controller.abort) and pass controller.signal to fetch; 2) run the per-message
uploads with controlled parallelism (e.g., collect promises and use
Promise.allSettled or a small concurrency pool) instead of strictly sequential
await to improve throughput; and 3) if any appendResponse is non-ok or any
promise rejects, cancel outstanding requests, and perform a cleanup pass that
removes previously inserted chunks (use the same keys created in
sessionStateSchema.chunks.insert, e.g., `${messageId}:0`) before throwing a
TRPCError, ensuring you still include the appendResponse.status/detail in the
thrown error.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 173-213: The menu currently allows re-importing the same Claude
session because the onSelect handler in the DropdownMenuItem (inside the
claudeSessions.map) always calls importClaudeSessionMutation; add a guard that
checks whether claudeSession.filePath is already imported (use whatever source
of truth exists: e.g., an importedSessions list, workspace sessions, or a new
Set of imported file paths) and if so either disable the DropdownMenuItem (set
disabled when filePath is present) or early-return in the onSelect handler with
toast.error("Session already imported") to prevent duplicate imports; update the
DropdownMenuItem props and the onSelect logic around
importClaudeSessionMutation.mutateAsync, and keep calls to onSelectSession and
setIsOpen only when a new import actually occurs.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 119-168: In listClaudeSessions, the loop that calls stat(filePath)
sequentially (iterating over candidates and pushing to withStats) creates
latency; replace it with a concurrent fan-out using Promise.allSettled over
candidates.map(({filePath, root}) => stat(filePath).then(fileStat => ({filePath,
fileStat, root}))). After Promise.allSettled, filter fulfilled results to
populate withStats and ignore rejected entries (or log them), preserving the
same shape used later; ensure you still return early when withStats is empty and
keep references to candidates, filePath, root, fileStat unchanged.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 88-90: The current requiresMainRestart boolean (based on
claudeSessionsErrorMessage substring checks) is fragile; change the code to
avoid string-matching by surfacing a typed error or sentinel from the
listSessions call and then checking that structured value (e.g., an error.code
or error.type such as "ROUTE_NOT_REGISTERED") instead of matching "No
\"query\"-procedure"/"No procedure"; update the listSessions procedure to return
a clear error shape or sentinel and then update the requiresMainRestart logic
(which currently references requiresMainRestart and claudeSessionsErrorMessage)
to inspect that structured error (or tRPC error.code) so the renderer can
reliably detect the "route not registered" case.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/desktop/src/lib/trpc/routers/chat-service/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx
Comment on lines
+122
to
+126
| }): Promise<ClaudeSessionSummary[]> { | ||
| const { cwd, limit } = args; | ||
| const roots = getClaudeSessionRoots().filter((root) => existsSync(root.rootDir)); | ||
| if (roots.length === 0) return []; | ||
|
|
Contributor
There was a problem hiding this comment.
CI formatter failure — fix formatting.
The pipeline flags lines 122–126 as incorrectly formatted. Run bunx biome check --write on this file before merging.
🧰 Tools
🪛 GitHub Actions: CI
[error] 122-126: Formatter would have printed the following content: fix formatting (prettier).
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 122 -
126, The CI formatter failure means this file's whitespace/formatting doesn't
match project style; run the project's formatter (bunx biome check --write) on
apps/desktop/src/lib/trpc/routers/chat-service/index.ts or manually reformat the
block around the function that returns Promise<ClaudeSessionSummary[]> (the
lines using const { cwd, limit } = args; const roots =
getClaudeSessionRoots().filter((root) => existsSync(root.rootDir)); if
(roots.length === 0) return [];), ensuring spacing and line breaks conform to
biome rules so the CI check passes.
Comment on lines
+258
to
+268
| const sessionFileContent = await readFile(normalizedFilePath, "utf8"); | ||
| const converted = convertExternalSessionToChatChunks({ | ||
| input: sessionFileContent, | ||
| providerId: "claude-code", | ||
| }); | ||
| if (converted.messages.length === 0) { | ||
| throw new TRPCError({ | ||
| code: "BAD_REQUEST", | ||
| message: "No importable messages found in Claude session", | ||
| }); | ||
| } |
Contributor
There was a problem hiding this comment.
convertExternalSessionToChatChunks is called without a try/catch — malformed JSONL throws a raw exception.
If the .jsonl file is corrupt or uses an unexpected format, the conversion will throw and the tRPC mutation will surface as a generic INTERNAL_SERVER_ERROR instead of a descriptive BAD_REQUEST. Wrap the call so the caller gets a meaningful error.
🛡️ Proposed fix
- const converted = convertExternalSessionToChatChunks({
- input: sessionFileContent,
- providerId: "claude-code",
- });
- if (converted.messages.length === 0) {
- throw new TRPCError({
- code: "BAD_REQUEST",
- message: "No importable messages found in Claude session",
- });
- }
+ let converted: ReturnType<typeof convertExternalSessionToChatChunks>;
+ try {
+ converted = convertExternalSessionToChatChunks({
+ input: sessionFileContent,
+ providerId: "claude-code",
+ });
+ } catch (err) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: `Failed to parse Claude session file: ${err instanceof Error ? err.message : String(err)}`,
+ });
+ }
+ if (converted.messages.length === 0) {
+ throw new TRPCError({
+ code: "BAD_REQUEST",
+ message: "No importable messages found in Claude session",
+ });
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 258 -
268, Wrap the call to convertExternalSessionToChatChunks in a try/catch so
malformed or unexpected .jsonl content produces a descriptive TRPCError instead
of bubbling a raw exception; catch errors around
convertExternalSessionToChatChunks (while keeping readFile as-is), and on
failure throw new TRPCError({ code: "BAD_REQUEST", message: `Invalid Claude
session file: ${error.message}` }) (optionally log the error) before proceeding
to the existing check for converted.messages.length === 0.
Comment on lines
+72
to
+81
| } = electronTrpc.chatServiceClaude.listSessions.useQuery( | ||
| { | ||
| cwd: cwd.trim().length > 0 ? cwd : "/", | ||
| limit: 20, | ||
| }, | ||
| { | ||
| enabled: isOpen, | ||
| staleTime: 30_000, | ||
| }, | ||
| ); |
Contributor
There was a problem hiding this comment.
Fallback cwd of "/" silently degrades to show all sessions.
When cwd is empty, encodeClaudeProjectPath("/") encodes to "-", which won't match any real project directory, so workspaceMatches in listClaudeSessions will be empty and the full allEntries fallback fires — potentially exposing sessions from unrelated projects. Since the PR notes the fallback behaviour is already a known risk (separate comment), at minimum document why "/" is chosen here or assert that cwd is always non-empty at the call site.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`
around lines 72 - 81, The query currently falls back to cwd = "/" which causes
encodeClaudeProjectPath("/") -> "-" and makes workspaceMatches empty so
listClaudeSessions returns allEntries; to fix, ensure the caller never passes an
empty cwd or fail-fast here: in the
electronTrpc.chatServiceClaude.listSessions.useQuery call (and related
session-selection flow) assert cwd is non-empty (e.g., return/throw or set
enabled: false when cwd.trim().length === 0) or replace the "/" fallback with a
clear sentinel (e.g., undefined) and/or add a comment documenting why "/" was
chosen and the security implication; reference encodeClaudeProjectPath,
workspaceMatches, listClaudeSessions and allEntries when making the change so
the behaviour is explicit.
Kitenite
force-pushed
the
kitenite/convert
branch
from
f4551e9 to
87c45a3
Compare
Kitenite
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (6)
apps/desktop/src/lib/trpc/routers/chat-service/index.ts (5)
258-262: WrapconvertExternalSessionToChatChunksin try-catch for malformed input.If the
.jsonlfile is corrupt or unexpected, the conversion throws an unhandled exception that surfaces as a genericINTERNAL_SERVER_ERROR. Wrapping in try-catch and re-throwing as aTRPCErrorwith codeBAD_REQUESTgives the caller a meaningful error.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 258 - 262, The call to convertExternalSessionToChatChunks can throw on malformed sessionFileContent; wrap the conversion in a try-catch around convertExternalSessionToChatChunks({ input: sessionFileContent, providerId: "claude-code" }) and on error throw a trpc TRPCError with code 'BAD_REQUEST' and a clear message (include the original error as cause or in the message) so callers receive a meaningful validation error instead of an INTERNAL_SERVER_ERROR.
411-441:listSessionsusespublicProcedure— no auth check.
importSessionvalidates auth insideimportClaudeSessionviagetAuthHeaders(), butlistSessionshas no authentication. This allows any renderer context to enumerate Claude session files on disk without being signed in.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 411 - 441, The listSessions endpoint in createChatServiceClaudeRouter is exposed with publicProcedure allowing unauthenticated enumeration of Claude session files; change listSessions to require authentication (e.g., use the same authenticated procedure used for importSession such as protectedProcedure or validate auth at the start of listClaudeSessions) so that only signed-in contexts can call it, and ensure the implementation uses the same auth guard or getAuthHeaders() pattern as importSession to enforce access control for listClaudeSessions.
87-95:encodeClaudeProjectPathstrips Windows drive letters instead of converting them.Claude's directory naming preserves the drive letter and converts the colon to a hyphen (e.g.,
C:\Users\...→C--Users-...), but line 89 strips the drive prefix entirely. This will cause workspace-scoped listing to miss sessions on Windows.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 87 - 95, The function encodeClaudeProjectPath currently removes the Windows drive prefix by using withoutDrive = normalized.replace(/^[A-Za-z]:/, "") which strips the drive letter; instead preserve and convert the drive colon into a hyphen so Windows paths map like "C:\Users..." → "C--Users-...". Modify encodeClaudeProjectPath to keep the leading drive letter (use the normalized value rather than withoutDrive), replace the drive colon (:) with a hyphen before splitting, and then apply the same segment sanitization (segment.replace(/[^A-Za-z0-9_-]/g, "-")) so the returned string includes the converted drive prefix.
233-250: Path validation usesfindClaudeSessionRootForPathbut does not resolve symlinks first.
importClaudeSessionreceivesfilePathfrom user input, resolves it viapath.resolve, and checks containment textually. If the resolved path is a symlink pointing outside the Claude directories, the containment check passes but the actual file read follows the symlink. Consider usingfs.realpathbefore the containment check.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 233 - 250, The path containment check in importClaudeSession currently uses path.resolve but not symlink resolution, so call fs.realpath (or fs.realpathSync) on the incoming filePath and use that real path for root discovery and all subsequent reads; update normalizedFilePath (and any usage of findClaudeSessionRootForPath) to use the realpath result so symlinks pointing outside the Claude directories are rejected before any file access.
294-360: Sequential per-message POST with no timeout — slow imports and no partial-failure recovery.Each converted message is appended via a separate
fetchPOST. For sessions with many messages this will be slow. Additionally, there is noAbortSignal/ timeout — a hung API blocks the Electron main process. If a POST fails at message N, the session exists with messages 0..N-1 but the mutation throws with no way to resume or clean up.Consider batching messages, adding
AbortSignal.timeout(ms), and handling partial failures.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (1)
68-81: Fallbackcwdof"/"when empty can expose unrelated project sessions.When
cwdis empty, the fallback to"/"causesencodeClaudeProjectPathto produce"-", which won't match any real project — triggering the all-sessions fallback in the backend.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 68 - 81, The query currently passes cwd fallback "/" which leads encodeClaudeProjectPath to produce "-" and causes the backend to return all sessions; instead, change the parameter passed to electronTrpc.chatServiceClaude.listSessions.useQuery so that when cwd.trim().length === 0 you pass undefined (or omit the project path) rather than "/". Update the call site around electronTrpc.chatServiceClaude.listSessions.useQuery (the object with cwd and limit) to send cwd only if non-empty (e.g., compute a projectPath from cwd and pass projectPath || undefined) so the backend won't treat empty workspaces as the root project.
🧹 Nitpick comments (6)
packages/chat/src/shared/session-conversion.ts (3)
241-244:extractTimestampis called twice on the same input when the condition is true.In both
codexSessionConverter.convertandclaudeCodeSessionConverter.convert, the sameextractTimestamp(message ?? {})call is evaluated twice — once in the condition check and again for the assignment. Cache the result in a local variable.♻️ Example fix (codex converter, same pattern applies to claude-code at lines 296-299)
- const createdAt = - extractTimestamp(message ?? {}) !== DEFAULT_TIMESTAMP - ? extractTimestamp(message ?? {}) - : extractTimestamp(record); + const messageTimestamp = extractTimestamp(message ?? {}); + const createdAt = + messageTimestamp !== DEFAULT_TIMESTAMP + ? messageTimestamp + : extractTimestamp(record);Also applies to: 296-299
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 241 - 244, In codexSessionConverter.convert and claudeCodeSessionConverter.convert avoid calling extractTimestamp(message ?? {}) twice by caching its result in a local variable (e.g., const messageTs = extractTimestamp(message ?? {})), then use messageTs in the conditional and for the createdAt assignment and fall back to extractTimestamp(record) only if messageTs === DEFAULT_TIMESTAMP; update both occurrences (the one around the createdAt assignment) to use the cached variable.
255-279:claudeCodeSessionConverter.detectmay be overly broad for auto-detection.Any JSON record with a
uuidstring field and non-empty extractable text (line 273-278) will match Claude. Sinceuuidis a very common field name, this could produce false-positive detections when the registry auto-resolves a converter. The scoring mechanism inresolveConverterpartially mitigates this, but if both converters score similarly, the result may be unpredictable.Consider tightening the fallback heuristic — e.g., requiring
parentUuid/parent_uuid(which are more Claude-specific) rather thanuuidalone, or combininguuidwith another Claude-specific signal.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 255 - 279, The fallback heuristic in claudeCodeSessionConverter.detect is too permissive because it treats any record with uuid and non-empty extractText as Claude; tighten it by changing the final condition to require a Claude-specific signal (e.g., require typeof record.parentUuid === "string" || typeof record.parent_uuid === "string") instead of accepting uuid alone, or require uuid plus another Claude-specific field (like a Claude-specific type or message.role) before returning true; update the logic inside claudeCodeSessionConverter.detect (where extractText is used) so resolveConverter scoring is less likely to misclassify by ensuring only records with parentUuid/parent_uuid (or uuid+additional Claude signal) match as Claude.
415-416: Module-level mutable singleton — mutations leak across consumers.
defaultSessionConverterRegistryis a shared mutable instance. If any consumer calls.register()or.unregister()on it, the change affects every other import site. This is fine if all usage goes throughconvertExternalSessionToChatChunks, but the instance is exported and could be mutated unintentionally.Consider either not exporting the instance directly (only exposing the convenience function) or documenting the shared-mutable contract.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/chat/src/shared/session-conversion.ts` around lines 415 - 416, The module exposes a mutable singleton defaultSessionConverterRegistry which can be mutated via methods like register/unregister and leak changes across consumers; change the API so callers cannot mutate the shared instance directly — either stop exporting defaultSessionConverterRegistry and only export a conversion helper like convertExternalSessionToChatChunks (which internally creates/uses a local registry from createDefaultSessionConverterRegistry), or export a factory/getter that returns a fresh registry (e.g., createDefaultSessionConverterRegistry or getNewSessionConverterRegistry) and keep the module-level registry private; update all call sites to use the helper/factory and remove direct exports of defaultSessionConverterRegistry to prevent accidental global mutations.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx (2)
84-90: Fragile string-matching forrequiresMainRestartdetection.Checking for
"No \"query\"-procedure"and"No procedure"in the error message (lines 89-90) is brittle — these strings come from tRPC internals and could change between versions. Consider detecting the "route not found" condition via error code (e.g., tRPC'sNOT_FOUNDcode) rather than substring matching on the message.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 84 - 90, The current requiresMainRestart computation uses brittle substring matching on claudeSessionsErrorMessage; instead inspect the error object (claudeSessionsError) for a tRPC "NOT_FOUND" indication (e.g., check error.code or error.data?.code or use a TRPC client error type guard) and set requiresMainRestart when that NOT_FOUND code is present; keep a fallback to the existing message substring checks only if the structured code is not available. Update the logic around claudeSessionsError / claudeSessionsErrorMessage and the requiresMainRestart assignment so it relies on the error code first rather than message text.
176-206: No guard against re-importing the same Claude session.A user can click the same Claude session entry multiple times (or revisit the dropdown later) and create duplicate imported sessions each time. Consider disabling already-imported sessions or tracking which
filePaths have been imported.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx` around lines 176 - 206, Prevent duplicate imports by checking whether the claude session's filePath has already been imported before calling importClaudeSessionMutation; inside the onSelect handler used in claudeSessions.map, add a guard that looks up the filePath in a local state/set of importedFilePaths (or calls an isImported helper that queries existing sessions) and early-returns or shows a toast if already imported, and ensure DropdownMenuItem is disabled when importClaudeSessionMutation.isPending or the filePath exists in importedFilePaths; update the set (e.g., addImportedFilePath(filePath)) after a successful import in the try block (where onImportClaudeSession is called) so subsequent clicks are ignored.apps/desktop/src/lib/trpc/routers/chat-service/index.ts (1)
163-168: Emptycatchblock silently swallowsstaterrors — consider logging.If
statfails for reasons other than "file not found" (e.g., permission denied), the error is silently ignored. At minimum, logging a warning would help diagnose issues when sessions mysteriously don't appear in the list.♻️ Proposed fix
for (const { filePath, root } of candidates) { try { const fileStat = await stat(filePath); withStats.push({ filePath, fileStat, root }); - } catch {} + } catch (err) { + console.warn(`[claude] failed to stat session file: ${filePath}`, err); + } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 163 - 168, The empty catch on the loop over candidates ({ filePath, root } in candidates) swallows stat errors; update the try/catch around await stat(filePath) in the same block so that non-ENOENT errors are logged (e.g., use your logger or console.warn/error) including filePath and the caught error, while still ignoring file-not-found if desired; keep pushing to withStats only on success (the existing withStats.push({ filePath, fileStat, root }) in the try block) and do not change that success path.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 366-375: The PATCH request that updates the session title (the
fetch call using env.NEXT_PUBLIC_API_URL with sessionId and body { title:
title.trim() }) is silently swallowing failures via .catch(() => {}); change
this to surface failures by either logging the error (use an existing logger or
console.error) inside the catch or by propagating a boolean/flag back from the
surrounding handler to indicate title update failure so callers can react;
ensure you reference the same headers and only attempt the request when
title.trim().length > 0, and preserve existing success flow while
recording/reporting any fetch error for debugging/user feedback.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 39-43: formatSessionDate renders the sentinel DEFAULT_TIMESTAMP
("1970-01-01T00:00:00.000Z") as a real date; update formatSessionDate to detect
that sentinel (e.g., compare isoDate to DEFAULT_TIMESTAMP or parsed.getTime()
=== 0) and return a user-friendly placeholder (empty string or "—"/"Unknown
date") instead of formatting it; keep the existing NaN check and use the
function name formatSessionDate and the DEFAULT_TIMESTAMP sentinel to locate the
fix.
---
Duplicate comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 258-262: The call to convertExternalSessionToChatChunks can throw
on malformed sessionFileContent; wrap the conversion in a try-catch around
convertExternalSessionToChatChunks({ input: sessionFileContent, providerId:
"claude-code" }) and on error throw a trpc TRPCError with code 'BAD_REQUEST' and
a clear message (include the original error as cause or in the message) so
callers receive a meaningful validation error instead of an
INTERNAL_SERVER_ERROR.
- Around line 411-441: The listSessions endpoint in
createChatServiceClaudeRouter is exposed with publicProcedure allowing
unauthenticated enumeration of Claude session files; change listSessions to
require authentication (e.g., use the same authenticated procedure used for
importSession such as protectedProcedure or validate auth at the start of
listClaudeSessions) so that only signed-in contexts can call it, and ensure the
implementation uses the same auth guard or getAuthHeaders() pattern as
importSession to enforce access control for listClaudeSessions.
- Around line 87-95: The function encodeClaudeProjectPath currently removes the
Windows drive prefix by using withoutDrive = normalized.replace(/^[A-Za-z]:/,
"") which strips the drive letter; instead preserve and convert the drive colon
into a hyphen so Windows paths map like "C:\Users..." → "C--Users-...". Modify
encodeClaudeProjectPath to keep the leading drive letter (use the normalized
value rather than withoutDrive), replace the drive colon (:) with a hyphen
before splitting, and then apply the same segment sanitization
(segment.replace(/[^A-Za-z0-9_-]/g, "-")) so the returned string includes the
converted drive prefix.
- Around line 233-250: The path containment check in importClaudeSession
currently uses path.resolve but not symlink resolution, so call fs.realpath (or
fs.realpathSync) on the incoming filePath and use that real path for root
discovery and all subsequent reads; update normalizedFilePath (and any usage of
findClaudeSessionRootForPath) to use the realpath result so symlinks pointing
outside the Claude directories are rejected before any file access.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 68-81: The query currently passes cwd fallback "/" which leads
encodeClaudeProjectPath to produce "-" and causes the backend to return all
sessions; instead, change the parameter passed to
electronTrpc.chatServiceClaude.listSessions.useQuery so that when
cwd.trim().length === 0 you pass undefined (or omit the project path) rather
than "/". Update the call site around
electronTrpc.chatServiceClaude.listSessions.useQuery (the object with cwd and
limit) to send cwd only if non-empty (e.g., compute a projectPath from cwd and
pass projectPath || undefined) so the backend won't treat empty workspaces as
the root project.
---
Nitpick comments:
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts`:
- Around line 163-168: The empty catch on the loop over candidates ({ filePath,
root } in candidates) swallows stat errors; update the try/catch around await
stat(filePath) in the same block so that non-ENOENT errors are logged (e.g., use
your logger or console.warn/error) including filePath and the caught error,
while still ignoring file-not-found if desired; keep pushing to withStats only
on success (the existing withStats.push({ filePath, fileStat, root }) in the try
block) and do not change that success path.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`:
- Around line 84-90: The current requiresMainRestart computation uses brittle
substring matching on claudeSessionsErrorMessage; instead inspect the error
object (claudeSessionsError) for a tRPC "NOT_FOUND" indication (e.g., check
error.code or error.data?.code or use a TRPC client error type guard) and set
requiresMainRestart when that NOT_FOUND code is present; keep a fallback to the
existing message substring checks only if the structured code is not available.
Update the logic around claudeSessionsError / claudeSessionsErrorMessage and the
requiresMainRestart assignment so it relies on the error code first rather than
message text.
- Around line 176-206: Prevent duplicate imports by checking whether the claude
session's filePath has already been imported before calling
importClaudeSessionMutation; inside the onSelect handler used in
claudeSessions.map, add a guard that looks up the filePath in a local state/set
of importedFilePaths (or calls an isImported helper that queries existing
sessions) and early-returns or shows a toast if already imported, and ensure
DropdownMenuItem is disabled when importClaudeSessionMutation.isPending or the
filePath exists in importedFilePaths; update the set (e.g.,
addImportedFilePath(filePath)) after a successful import in the try block (where
onImportClaudeSession is called) so subsequent clicks are ignored.
In `@packages/chat/src/shared/session-conversion.ts`:
- Around line 241-244: In codexSessionConverter.convert and
claudeCodeSessionConverter.convert avoid calling extractTimestamp(message ?? {})
twice by caching its result in a local variable (e.g., const messageTs =
extractTimestamp(message ?? {})), then use messageTs in the conditional and for
the createdAt assignment and fall back to extractTimestamp(record) only if
messageTs === DEFAULT_TIMESTAMP; update both occurrences (the one around the
createdAt assignment) to use the cached variable.
- Around line 255-279: The fallback heuristic in
claudeCodeSessionConverter.detect is too permissive because it treats any record
with uuid and non-empty extractText as Claude; tighten it by changing the final
condition to require a Claude-specific signal (e.g., require typeof
record.parentUuid === "string" || typeof record.parent_uuid === "string")
instead of accepting uuid alone, or require uuid plus another Claude-specific
field (like a Claude-specific type or message.role) before returning true;
update the logic inside claudeCodeSessionConverter.detect (where extractText is
used) so resolveConverter scoring is less likely to misclassify by ensuring only
records with parentUuid/parent_uuid (or uuid+additional Claude signal) match as
Claude.
- Around line 415-416: The module exposes a mutable singleton
defaultSessionConverterRegistry which can be mutated via methods like
register/unregister and leak changes across consumers; change the API so callers
cannot mutate the shared instance directly — either stop exporting
defaultSessionConverterRegistry and only export a conversion helper like
convertExternalSessionToChatChunks (which internally creates/uses a local
registry from createDefaultSessionConverterRegistry), or export a factory/getter
that returns a fresh registry (e.g., createDefaultSessionConverterRegistry or
getNewSessionConverterRegistry) and keep the module-level registry private;
update all call sites to use the helper/factory and remove direct exports of
defaultSessionConverterRegistry to prevent accidental global mutations.
ℹ️ Review info
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/desktop/src/lib/trpc/routers/chat-service/index.tsapps/desktop/src/lib/trpc/routers/index.tsapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/ChatMastraPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsxapps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsxpackages/chat/src/shared/index.tspackages/chat/src/shared/session-conversion.test.tspackages/chat/src/shared/session-conversion.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/chat/src/shared/index.ts
- packages/chat/src/shared/session-conversion.test.ts
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatPane.tsx
- apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/components/SessionSelector/SessionSelector.tsx
Comment on lines
+366
to
+375
| if (title.trim().length > 0) { | ||
| await fetch(`${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}`, { | ||
| method: "PATCH", | ||
| headers: { | ||
| ...headers, | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ title: title.trim() }), | ||
| }).catch(() => {}); | ||
| } |
Contributor
There was a problem hiding this comment.
Title PATCH failure is silently swallowed — imported session may appear untitled.
The .catch(() => {}) on line 374 means if the title update fails (e.g., network hiccup), the imported session will have no title and the user gets no feedback. Since the import already succeeded at this point, consider at least logging the failure or returning a flag indicating the title wasn't set.
♻️ Proposed fix
if (title.trim().length > 0) {
await fetch(`${env.NEXT_PUBLIC_API_URL}/api/chat/${sessionId}`, {
method: "PATCH",
headers: {
...headers,
"Content-Type": "application/json",
},
body: JSON.stringify({ title: title.trim() }),
- }).catch(() => {});
+ }).catch((err) => {
+ console.warn(`[claude] failed to set imported session title: ${sessionId}`, err);
+ });
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@apps/desktop/src/lib/trpc/routers/chat-service/index.ts` around lines 366 -
375, The PATCH request that updates the session title (the fetch call using
env.NEXT_PUBLIC_API_URL with sessionId and body { title: title.trim() }) is
silently swallowing failures via .catch(() => {}); change this to surface
failures by either logging the error (use an existing logger or console.error)
inside the catch or by propagating a boolean/flag back from the surrounding
handler to indicate title update failure so callers can react; ensure you
reference the same headers and only attempt the request when title.trim().length
> 0, and preserve existing success flow while recording/reporting any fetch
error for debugging/user feedback.
Comment on lines
+39
to
+43
| function formatSessionDate(isoDate: string): string { | ||
| const parsed = new Date(isoDate); | ||
| if (Number.isNaN(parsed.getTime())) return isoDate; | ||
| return parsed.toLocaleString(); | ||
| } |
Contributor
There was a problem hiding this comment.
formatSessionDate renders the epoch fallback (1970-01-01) as a real date.
When the session conversion couldn't extract a timestamp, it falls back to DEFAULT_TIMESTAMP ("1970-01-01T00:00:00.000Z"). formatSessionDate would render this as "1/1/1970, 12:00:00 AM", which looks like a bug to users. Consider treating this sentinel value as a special case.
♻️ Proposed fix
function formatSessionDate(isoDate: string): string {
+ if (isoDate === "1970-01-01T00:00:00.000Z") return "Unknown date";
const parsed = new Date(isoDate);
if (Number.isNaN(parsed.getTime())) return isoDate;
return parsed.toLocaleString();
}📝 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.
Suggested change
| function formatSessionDate(isoDate: string): string { | |
| const parsed = new Date(isoDate); | |
| if (Number.isNaN(parsed.getTime())) return isoDate; | |
| return parsed.toLocaleString(); | |
| } | |
| function formatSessionDate(isoDate: string): string { | |
| if (isoDate === "1970-01-01T00:00:00.000Z") return "Unknown date"; | |
| const parsed = new Date(isoDate); | |
| if (Number.isNaN(parsed.getTime())) return isoDate; | |
| return parsed.toLocaleString(); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatMastraPane/components/SessionSelector/SessionSelector.tsx`
around lines 39 - 43, formatSessionDate renders the sentinel DEFAULT_TIMESTAMP
("1970-01-01T00:00:00.000Z") as a real date; update formatSessionDate to detect
that sentinel (e.g., compare isoDate to DEFAULT_TIMESTAMP or parsed.getTime()
=== 0) and return a user-friendly placeholder (empty string or "—"/"Unknown
date") instead of formatting it; keep the existing NaN check and use the
function name formatSessionDate and the DEFAULT_TIMESTAMP sentinel to locate the
fix.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mainchatServiceClauderouter for Claude list/import procedures (keeps corechatServicerouter isolated)Validation
bun run --cwd apps/desktop typecheckbun test packages/chat/src/shared/session-conversion.test.tsconvertExternalSessionToChatChunksagainst 8 real UUID Claude session files in~/.claude/projects(all parsed and converted without runtime errors)Notes
chatServiceClaudeSummary by CodeRabbit
New Features
Chores
Tests