feat(desktop): add Claude binary bundling and AI chat tRPC router#1204
Conversation
- Add download/upgrade scripts for Claude Code binary - Configure Git LFS for darwin-arm64 binary - Bundle binary via electron-builder extraResources - Add AI chat tRPC router with auth and session management - Add @anthropic-ai/claude-agent-sdk dependency
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughAdds local Claude Code support: LFS tracking and packaging for a darwin‑arm64 binary, download/upgrade scripts with manifest/hash verification, credential resolution, a ClaudeSessionManager that streams and persists SDK messages to durable streams, and a new tRPC aiChat router exposing session controls and event streaming. Changes
Sequence DiagramsequenceDiagram
participant Client as Client
participant tRPC as tRPC Router
participant Auth as Auth Module
participant Manager as ClaudeSessionManager
participant SDK as Claude CLI/SDK
participant Stream as Durable Stream
Client->>tRPC: startSession(sessionId, cwd)
tRPC->>Auth: getExistingClaudeCredentials()
Auth-->>tRPC: credentials / env
tRPC->>Manager: startSession(sessionId, cwd, claudeSessionId?)
Manager->>SDK: query(streaming, includePartialMessages)
SDK->>Manager: partial/full SDK messages
Manager->>Stream: persist JSON chunks (with _seq, timestamps)
Stream-->>Manager: ack
Manager->>tRPC: emit session_start / events
tRPC->>Client: streamEvents (observable)
Client->>tRPC: interrupt/stopSession(sessionId)
tRPC->>Manager: interrupt/stopSession
Manager->>SDK: interrupt/close
Manager->>Stream: finalize / cleanup
Manager-->>tRPC: session_end
tRPC-->>Client: notified
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 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 |
🚀 Preview Deployment🔗 Preview Links
Preview updates automatically with new commits |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Fix all issues with AI agents
In `@apps/desktop/scripts/download-claude-binary.ts`:
- Around line 121-190: The downloadFile function currently creates the write
stream upfront and reuses/closess it across redirects causing res.pipe(file) to
write into a closed stream; change downloadFile/request so the
createWriteStream(destPath) is created only after receiving a 200 response
(inside the HTTPS callback), and move all file event handlers
(finish/error/close) and piping there; on 301/302 just follow the redirect
without closing or reusing a shared stream (clean up any partial file before
starting the new stream), and keep cleanup logic (unlinkSync destPath) on
non-200 errors and request-level errors; reference downloadFile, request(),
res.pipe(file), and the file variable when making these changes.
- Around line 307-318: The manifest fetched at manifestUrl and assigned to
manifest via fetchJson<Manifest> must be runtime-validated instead of only
relying on the TypeScript Manifest type; add a Zod schema (or equivalent) for
the expected manifest shape and validate the fetched payload (the value assigned
to manifest) immediately after fetchJson<Manifest>(manifestUrl),
throwing/logging a clear error and calling process.exit(1) when
parsing/validation fails; update references to the manifest variable usage to
assume the validated/parsed output (e.g., validatedManifest) so downstream code
uses a guaranteed shape and unknown/missing fields are rejected at load time.
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 41-56: The interrupt and stopSession procedures currently always
return success even if the session doesn't exist; update both
publicProcedure.mutation handlers (interrupt and stopSession) to call
claudeSessionManager.isSessionActive({ sessionId: input.sessionId }) before
invoking interrupt/stopSession, and if it returns false throw a TRPCError({
code: 'NOT_FOUND', message: 'Session not found' }) instead of proceeding; keep
the existing await calls to claudeSessionManager.interrupt/stopSession and
return { success: true } only after the existence check passes.
In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts`:
- Around line 15-81: buildClaudeEnv currently ignores any credentials where
source === "config", which drops valid API keys from config files; change the
credential shape and selection so we only skip OAuth-style tokens. Update
ClaudeCredentials (or add a new field) to distinguish kind: "apiKey" | "oauth";
in getCredentialsFromConfig return kind:"oauth" when you detect
config.claudeAiOauth?.accessToken (or oauthAccessToken fields) and return
kind:"apiKey" when you find config.apiKey / api_key / oauthAccessToken
(non-OAuth CLI) values; then update buildClaudeEnv to accept credentials with
kind === "apiKey" and only skip/handle differently when kind === "oauth".
Reference symbols: ClaudeCredentials, ClaudeConfigFile,
getCredentialsFromConfig, claudeAiOauth, and buildClaudeEnv.
- Around line 202-214: The PATH handling currently hardcodes ":" which breaks on
Windows; update the logic in this block (symbols: pathAdditions, currentPath,
pathParts, env.PATH) to use the platform delimiter from node:path (import {
delimiter } from "node:path") and replace the ":" used in split and join with
delimiter so splitting and rejoining of currentPath works cross-platform.
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`:
- Around line 314-316: The console.log in processUserMessage that prints
`content.slice(0, 50)` leaks user prompt data; change the logging in
session-manager.ts (inside the processUserMessage function for the given
sessionId) to avoid printing prompt text and instead log a non-sensitive
descriptor such as the content length or a deterministic hash (e.g., sha256)
plus sessionId, e.g., replace the current message with one that includes
sessionId and either `content.length` or a hash of `content` so no raw prompt
text is emitted.
- Around line 173-212: The poll() method currently swallows errors in its catch
block; update the catch to log the error with context (include this.sessionId
and a short message like "poll error for session") using the instance logger if
present (e.g., this.logger.error) or console.error as a fallback, and then
handle or rethrow as appropriate (at minimum do not silently ignore the error).
Refer to the poll method and the this.sessionId / this.isPolling symbols when
making the change.
🧹 Nitpick comments (7)
apps/desktop/scripts/download-claude-binary.ts (3)
121-232: Prefer object parameters for multi-arg helpers.
downloadFile(url, destPath)anddownloadPlatform(version, platformKey, manifest)use positional parameters. The project guideline asks for object parameters for 2+ args to avoid positional/ordering mistakes.♻️ Example refactor pattern
-function downloadPlatform( - version: string, - platformKey: string, - manifest: Manifest, -): Promise<boolean> { +function downloadPlatform({ + version, + platformKey, + manifest, +}: { + version: string; + platformKey: string; + manifest: Manifest; +}): Promise<boolean> {-const result = await downloadPlatform(version, platform, manifest); +const result = await downloadPlatform({ version, platformKey: platform, manifest });
209-220: Prefix logs with a consistent context tag.Please align the logs with the
[domain/operation] messagepattern (e.g.,[claude/download] Fetching latest version…) for entry/exit, external calls, and errors.
221-223: Extract fallback version into a named constant.The hardcoded fallback
"2.1.17"should be lifted to a top-level constant for visibility and easy update.apps/desktop/scripts/upgrade-claude-binary.ts (2)
25-31: Use an object parameter forrunoptions.This helper has 2 parameters; please switch to a single object parameter to align with the guideline.
♻️ Suggested refactor
-function run(cmd: string, opts?: { cwd?: string }): string { +function run({ cmd, cwd }: { cmd: string; cwd?: string }): string { console.log(`$ ${cmd}`); return execSync(cmd, { encoding: "utf-8", - cwd: opts?.cwd ?? DESKTOP_DIR, + cwd: cwd ?? DESKTOP_DIR, stdio: ["inherit", "pipe", "inherit"], }).trim(); }-run(`bun run scripts/download-claude-binary.ts${versionFlag}`); +run({ cmd: `bun run scripts/download-claude-binary.ts${versionFlag}` });
39-76: Prefix logs with a consistent context tag.Please use the
[domain/operation] messagepattern (e.g.,[claude/upgrade] Staging binary…) for significant steps and errors to match the logging guideline.apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts (2)
113-118: Prefer object params forStreamWatchercallbacks.The callback currently uses two positional args; switching to an object keeps call sites self-documenting and matches repo guidance. As per coding guidelines: Use object parameters for functions with 2 or more parameters instead of positional arguments.
♻️ Suggested refactor
class StreamWatcher { private intervalId: ReturnType<typeof setInterval> | null = null; private seenMessageIds: Set<string> = new Set(); private isPolling = false; private isStopped = false; - private onNewUserMessage: (messageId: string, content: string) => void; + private onNewUserMessage: (input: { + messageId: string; + content: string; + }) => void; private sessionId = ""; - constructor(onNewUserMessage: (messageId: string, content: string) => void) { + constructor( + onNewUserMessage: (input: { messageId: string; content: string }) => void, + ) { this.onNewUserMessage = onNewUserMessage; } @@ - this.onNewUserMessage(key, content); + this.onNewUserMessage({ messageId: key, content }); } } @@ - const watcher = new StreamWatcher((messageId, content) => { + const watcher = new StreamWatcher(({ messageId, content }) => { if (session.processingMessageIds.has(messageId)) { return; }Also applies to: 281-289
133-137: Extract polling interval and default model into constants.Inline
500and the model string are magic values; lift them to module-level constants for clarity and reuse. As per coding guidelines: Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.♻️ Suggested refactor
const DURABLE_STREAM_URL = process.env.DURABLE_STREAM_URL || "http://localhost:8080"; +const STREAM_POLL_INTERVAL_MS = 500; +const DEFAULT_MODEL = "claude-sonnet-4-5-20250929"; @@ - this.intervalId = setInterval(() => this.poll(), 500); + this.intervalId = setInterval(() => this.poll(), STREAM_POLL_INTERVAL_MS); @@ - model: process.env.CLAUDE_MODEL || "claude-sonnet-4-5-20250929", + model: process.env.CLAUDE_MODEL || DEFAULT_MODEL,Also applies to: 362-367
| function downloadFile(url: string, destPath: string): Promise<void> { | ||
| return new Promise((resolve, reject) => { | ||
| // Ensure parent directory exists | ||
| const parentDir = dirname(destPath); | ||
| if (!existsSync(parentDir)) { | ||
| mkdirSync(parentDir, { recursive: true }); | ||
| } | ||
|
|
||
| const file = createWriteStream(destPath); | ||
|
|
||
| const request = (requestUrl: string) => { | ||
| https | ||
| .get(requestUrl, (res) => { | ||
| if (res.statusCode === 301 || res.statusCode === 302) { | ||
| file.close(); | ||
| if (existsSync(destPath)) unlinkSync(destPath); | ||
| const location = res.headers.location; | ||
| if (location) { | ||
| return request(location); | ||
| } | ||
| return reject(new Error("Redirect without location")); | ||
| } | ||
|
|
||
| if (res.statusCode !== 200) { | ||
| file.close(); | ||
| if (existsSync(destPath)) unlinkSync(destPath); | ||
| return reject(new Error(`HTTP ${res.statusCode}`)); | ||
| } | ||
|
|
||
| const totalSize = Number.parseInt( | ||
| res.headers["content-length"] || "0", | ||
| 10, | ||
| ); | ||
| let downloaded = 0; | ||
| let lastPercent = 0; | ||
|
|
||
| res.on("data", (chunk: Buffer) => { | ||
| downloaded += chunk.length; | ||
| if (totalSize > 0) { | ||
| const percent = Math.floor((downloaded / totalSize) * 100); | ||
| if (percent !== lastPercent && percent % 10 === 0) { | ||
| process.stdout.write(`\r Progress: ${percent}%`); | ||
| lastPercent = percent; | ||
| } | ||
| } | ||
| }); | ||
|
|
||
| res.pipe(file); | ||
|
|
||
| file.on("finish", () => { | ||
| file.close(); | ||
| process.stdout.write("\r Progress: 100%\n"); | ||
| resolve(); | ||
| }); | ||
|
|
||
| res.on("error", (err) => { | ||
| file.close(); | ||
| if (existsSync(destPath)) unlinkSync(destPath); | ||
| reject(err); | ||
| }); | ||
| }) | ||
| .on("error", (err) => { | ||
| file.close(); | ||
| if (existsSync(destPath)) unlinkSync(destPath); | ||
| reject(err); | ||
| }); | ||
| }; | ||
|
|
||
| request(url); | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
head -200 apps/desktop/scripts/download-claude-binary.ts | tail -80Repository: superset-sh/superset
Length of output: 2180
Fix redirect handling to avoid piping into a closed stream.
downloadFile() creates the write stream once at the start, then closes and reuses it on redirects. When a 301/302 redirect occurs, the stream is closed but the file variable persists in the closure scope of the request() function. On the recursive call, the new response attempts to res.pipe(file) to the already-closed stream, which will fail or corrupt the download. Create the write stream only after receiving a 200 response.
🔧 Suggested fix
- const file = createWriteStream(destPath);
-
const request = (requestUrl: string) => {
https
.get(requestUrl, (res) => {
if (res.statusCode === 301 || res.statusCode === 302) {
- file.close();
- if (existsSync(destPath)) unlinkSync(destPath);
+ if (existsSync(destPath)) unlinkSync(destPath);
const location = res.headers.location;
if (location) {
return request(location);
}
return reject(new Error("Redirect without location"));
}
if (res.statusCode !== 200) {
- file.close();
if (existsSync(destPath)) unlinkSync(destPath);
return reject(new Error(`HTTP ${res.statusCode}`));
}
+ const file = createWriteStream(destPath);
const totalSize = Number.parseInt(
res.headers["content-length"] || "0",
10,
);
@@
res.pipe(file);
file.on("finish", () => {
file.close();
process.stdout.write("\r Progress: 100%\n");
resolve();
});
+ file.on("error", (err) => {
+ file.close();
+ if (existsSync(destPath)) unlinkSync(destPath);
+ reject(err);
+ });
res.on("error", (err) => {
file.close();
if (existsSync(destPath)) unlinkSync(destPath);
reject(err);
});🤖 Prompt for AI Agents
In `@apps/desktop/scripts/download-claude-binary.ts` around lines 121 - 190, The
downloadFile function currently creates the write stream upfront and
reuses/closess it across redirects causing res.pipe(file) to write into a closed
stream; change downloadFile/request so the createWriteStream(destPath) is
created only after receiving a 200 response (inside the HTTPS callback), and
move all file event handlers (finish/error/close) and piping there; on 301/302
just follow the redirect without closing or reusing a shared stream (clean up
any partial file before starting the new stream), and keep cleanup logic
(unlinkSync destPath) on non-200 errors and request-level errors; reference
downloadFile, request(), res.pipe(file), and the file variable when making these
changes.
| const manifestUrl = `${DIST_BASE}/${version}/manifest.json`; | ||
| console.log(`Fetching manifest: ${manifestUrl}`); | ||
|
|
||
| let manifest: Manifest; | ||
| try { | ||
| manifest = await fetchJson<Manifest>(manifestUrl); | ||
| } catch (error) { | ||
| console.error( | ||
| `Failed to fetch manifest: ${error instanceof Error ? error.message : error}`, | ||
| ); | ||
| process.exit(1); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd download-claude-binary.tsRepository: superset-sh/superset
Length of output: 110
🏁 Script executed:
cat -n apps/desktop/scripts/download-claude-binary.ts | sed -n '300,330p'Repository: superset-sh/superset
Length of output: 1236
🏁 Script executed:
# Check the Manifest type definition
rg "type Manifest|interface Manifest" apps/desktop/scripts/Repository: superset-sh/superset
Length of output: 131
🏁 Script executed:
# Check if Zod is in the project
rg "zod" --type json | head -20Repository: superset-sh/superset
Length of output: 722
🏁 Script executed:
# Look for fetchJson implementation
rg "function fetchJson|const fetchJson" apps/desktop/scripts/Repository: superset-sh/superset
Length of output: 159
🏁 Script executed:
# Check if validation is done elsewhere in the script
rg "z\." apps/desktop/scripts/download-claude-binary.tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Get the Manifest interface definition
rg -A 15 "interface Manifest" apps/desktop/scripts/download-claude-binary.tsRepository: superset-sh/superset
Length of output: 413
🏁 Script executed:
# Get fetchJson implementation
rg -B 2 -A 10 "function fetchJson" apps/desktop/scripts/download-claude-binary.tsRepository: superset-sh/superset
Length of output: 497
🏁 Script executed:
# See how manifest is used after fetching
rg -A 20 "manifest = await fetchJson" apps/desktop/scripts/download-claude-binary.tsRepository: superset-sh/superset
Length of output: 732
🏁 Script executed:
# Check for Zod usage in desktop app
rg "z\." apps/desktop/ --type ts | head -30Repository: superset-sh/superset
Length of output: 2805
🏁 Script executed:
# Check how external API data is validated elsewhere in codebase
rg -B 2 -A 5 "z\.object\|z\.record\|z\.string" apps/desktop/ --type ts | head -40Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Get PlatformManifest definition
rg -B 2 -A 10 "interface PlatformManifest" apps/desktop/scripts/download-claude-binary.tsRepository: superset-sh/superset
Length of output: 257
🏁 Script executed:
# See full usage of manifest object throughout the script
rg "manifest\." apps/desktop/scripts/download-claude-binary.ts -A 2 -B 1Repository: superset-sh/superset
Length of output: 379
🏁 Script executed:
# Check if coding guideline about external API validation applies to desktop scripts
cat apps/desktop/package.json | head -30Repository: superset-sh/superset
Length of output: 1439
Validate the manifest payload before using it.
The manifest JSON is fetched from an external URL and currently trusted via TypeScript interface only. Add runtime validation (e.g., Zod) to handle missing fields, unknown values, and shape drift, failing explicitly when the data is malformed.
🧪 Suggested fix (Zod validation)
+import { z } from "zod";
@@
+const ManifestSchema = z.object({
+ version: z.string(),
+ platforms: z.record(
+ z.object({
+ checksum: z.string(),
+ size: z.number().int().nonnegative(),
+ }),
+ ),
+});
+
@@
- manifest = await fetchJson<Manifest>(manifestUrl);
+ const rawManifest = await fetchJson<Manifest>(manifestUrl);
+ manifest = ManifestSchema.parse(rawManifest);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const manifestUrl = `${DIST_BASE}/${version}/manifest.json`; | |
| console.log(`Fetching manifest: ${manifestUrl}`); | |
| let manifest: Manifest; | |
| try { | |
| manifest = await fetchJson<Manifest>(manifestUrl); | |
| } catch (error) { | |
| console.error( | |
| `Failed to fetch manifest: ${error instanceof Error ? error.message : error}`, | |
| ); | |
| process.exit(1); | |
| } | |
| import { z } from "zod"; | |
| const ManifestSchema = z.object({ | |
| version: z.string(), | |
| platforms: z.record( | |
| z.object({ | |
| checksum: z.string(), | |
| size: z.number().int().nonnegative(), | |
| }), | |
| ), | |
| }); | |
| // ... (other code) | |
| const manifestUrl = `${DIST_BASE}/${version}/manifest.json`; | |
| console.log(`Fetching manifest: ${manifestUrl}`); | |
| let manifest: Manifest; | |
| try { | |
| const rawManifest = await fetchJson<Manifest>(manifestUrl); | |
| manifest = ManifestSchema.parse(rawManifest); | |
| } catch (error) { | |
| console.error( | |
| `Failed to fetch manifest: ${error instanceof Error ? error.message : error}`, | |
| ); | |
| process.exit(1); | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/scripts/download-claude-binary.ts` around lines 307 - 318, The
manifest fetched at manifestUrl and assigned to manifest via fetchJson<Manifest>
must be runtime-validated instead of only relying on the TypeScript Manifest
type; add a Zod schema (or equivalent) for the expected manifest shape and
validate the fetched payload (the value assigned to manifest) immediately after
fetchJson<Manifest>(manifestUrl), throwing/logging a clear error and calling
process.exit(1) when parsing/validation fails; update references to the manifest
variable usage to assume the validated/parsed output (e.g., validatedManifest)
so downstream code uses a guaranteed shape and unknown/missing fields are
rejected at load time.
| interrupt: publicProcedure | ||
| .input(z.object({ sessionId: z.string() })) | ||
| .mutation(async ({ input }) => { | ||
| await claudeSessionManager.interrupt({ sessionId: input.sessionId }); | ||
| return { success: true }; | ||
| }), | ||
|
|
||
| /** | ||
| * Stop a Claude session completely. | ||
| */ | ||
| stopSession: publicProcedure | ||
| .input(z.object({ sessionId: z.string() })) | ||
| .mutation(async ({ input }) => { | ||
| await claudeSessionManager.stopSession({ sessionId: input.sessionId }); | ||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -name "index.ts" -path "*/ai-chat/*" | head -20Repository: superset-sh/superset
Length of output: 255
🏁 Script executed:
find . -name "*claude*manager*" -o -name "*session*manager*" | grep -E "\.(ts|tsx)$" | head -20Repository: superset-sh/superset
Length of output: 148
🏁 Script executed:
git ls-files | grep -E "apps/desktop.*ai-chat" | head -20Repository: superset-sh/superset
Length of output: 393
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/ai-chat/index.tsRepository: superset-sh/superset
Length of output: 118
🏁 Script executed:
wc -l apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.tsRepository: superset-sh/superset
Length of output: 150
🏁 Script executed:
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/index.tsRepository: superset-sh/superset
Length of output: 3319
🏁 Script executed:
sed -n '1,100p' apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts | cat -nRepository: superset-sh/superset
Length of output: 3844
🏁 Script executed:
rg -n "interrupt|stopSession|isSessionActive" apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts -A 10 | head -80Repository: superset-sh/superset
Length of output: 1674
🏁 Script executed:
rg "TRPCError" apps/desktop/src/lib/trpc/ -B 1 -A 2 | head -40Repository: superset-sh/superset
Length of output: 3238
🏁 Script executed:
rg "from.*@trpc/server" apps/desktop/src/lib/trpc/ | head -10Repository: superset-sh/superset
Length of output: 1104
Return NOT_FOUND when interrupt/stopSession targets don't exist.
The interrupt and stopSession procedures always return success regardless of whether the session ID is valid. The session manager silently handles missing sessions without error signaling, creating ambiguous client state. Add a pre-check using the existing isSessionActive() method and throw a TRPCError with code NOT_FOUND before delegating to the manager, consistent with the TRPCError usage guidelines.
Suggested fix
-import { observable } from "@trpc/server/observable";
+import { TRPCError } from "@trpc/server";
+import { observable } from "@trpc/server/observable";
import { z } from "zod";
import { publicProcedure, router } from "../..";
@@
interrupt: publicProcedure
.input(z.object({ sessionId: z.string() }))
.mutation(async ({ input }) => {
+ if (!claudeSessionManager.isSessionActive(input.sessionId)) {
+ throw new TRPCError({
+ code: "NOT_FOUND",
+ message: "Session not found",
+ });
+ }
await claudeSessionManager.interrupt({ sessionId: input.sessionId });
return { success: true };
}),
@@
stopSession: publicProcedure
.input(z.object({ sessionId: z.string() }))
.mutation(async ({ input }) => {
+ if (!claudeSessionManager.isSessionActive(input.sessionId)) {
+ throw new TRPCError({
+ code: "NOT_FOUND",
+ message: "Session not found",
+ });
+ }
await claudeSessionManager.stopSession({ sessionId: input.sessionId });
return { success: true };
}),🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 41 - 56, The
interrupt and stopSession procedures currently always return success even if the
session doesn't exist; update both publicProcedure.mutation handlers (interrupt
and stopSession) to call claudeSessionManager.isSessionActive({ sessionId:
input.sessionId }) before invoking interrupt/stopSession, and if it returns
false throw a TRPCError({ code: 'NOT_FOUND', message: 'Session not found' })
instead of proceeding; keep the existing await calls to
claudeSessionManager.interrupt/stopSession and return { success: true } only
after the existence check passes.
# Conflicts: # .gitignore
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ai-chat-plan.md (1)
306-329:⚠️ Potential issue | 🟡 MinorSpecify a language for the fenced code block.
markdownlint MD040 warns when a fenced block lacks a language identifier.✏️ Proposed fix
-``` +```text apps/streams/ ├── package.json ├── Dockerfile ├── fly.toml └── src/ ├── index.ts # Hono server ├── streams.ts # Durable Stream handlers └── presence.ts # State Protocol handlers</details> </blockquote></details> </blockquote></details>🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts`: - Around line 112-150: In getCredentialsFromKeychain, replace the empty catch blocks with catches that log the unexpected error with a clear prefix and the error details (e.g., catch (err) { console.error("[claude/auth] keychain lookup error:", err); }) so failures during credential lookup/parse are diagnosable; do the same for the second keychain attempt inside getCredentialsFromKeychain and apply the identical logging change to the other credential lookup block referenced around lines 237-254 so no credential-related errors are swallowed silently. In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`: - Around line 172-178: The onError callback in the stream watcher (the onError handler that logs `[stream-watcher] Stream error for ${this.sessionId}`) currently returns an empty object `{}`; either remove that return so the handler returns void/undefined, or if the `@durable-streams/client` API requires a specific return value, replace the `{}` with the correct typed return and add a short comment explaining why that value is required; update the onError in session-manager.ts (the stream watcher callback) and include a one-line comment referencing `@durable-streams/client` behavior if you keep a non-void return.🧹 Nitpick comments (6)
apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts (2)
49-107: Extract config path list to module-level constants.
Keeps the function focused on resolution logic and avoids inline magic strings.As per coding guidelines, “Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.”♻️ Suggested refactor
+const CLAUDE_HOME = homedir(); +const CLAUDE_CONFIG_PATHS = [ + join(CLAUDE_HOME, ".claude", ".credentials.json"), + join(CLAUDE_HOME, ".claude.json"), + join(CLAUDE_HOME, ".config", "claude", "credentials.json"), + join(CLAUDE_HOME, ".config", "claude", "config.json"), +]; + function getCredentialsFromConfig(): ClaudeCredentials | null { - const home = homedir(); - // Check Claude Code CLI credentials first (most common case) - const configPaths = [ - join(home, ".claude", ".credentials.json"), // Claude Code CLI - join(home, ".claude.json"), - join(home, ".config", "claude", "credentials.json"), - join(home, ".config", "claude", "config.json"), - ]; - - for (const configPath of configPaths) { + // Check Claude Code CLI credentials first (most common case) + for (const configPath of CLAUDE_CONFIG_PATHS) { if (existsSync(configPath)) { try {
195-233: Move PATH additions and entrypoint string to constants.
Reduces inline magic strings and keeps the function focused.As per coding guidelines, “Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.”♻️ Suggested refactor
+const PATH_ADDITIONS = ["/usr/local/bin", "/opt/homebrew/bin", "/usr/bin"]; +const CLAUDE_CODE_ENTRYPOINT = "sdk-ts"; + export function buildClaudeEnv(): Record<string, string> { @@ if (platform() !== "win32") { - const pathAdditions = ["/usr/local/bin", "/opt/homebrew/bin", "/usr/bin"]; const currentPath = env.PATH || ""; const pathParts = currentPath.split(delimiter); - for (const addition of pathAdditions) { + for (const addition of PATH_ADDITIONS) { if (!pathParts.includes(addition)) { pathParts.push(addition); } } @@ - env.CLAUDE_CODE_ENTRYPOINT = "sdk-ts"; + env.CLAUDE_CODE_ENTRYPOINT = CLAUDE_CODE_ENTRYPOINT;apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts (4)
91-96: Log the fallback path when stream creation fails.The empty catch block silently swallows the error. While the fallback to
connect()is intentional, logging provides visibility into whether streams are being created or connected to existing ones.🔧 Suggested fix
try { stream = await DurableStream.create(streamOpts); - } catch { + } catch (error) { + console.log( + `[durable-stream] Stream ${sessionId} already exists, connecting:`, + error instanceof Error ? error.message : error, + ); // Stream may already exist — connect to it stream = await DurableStream.connect(streamOpts); }Based on learnings: Never swallow errors silently; at minimum log errors with context before rethrowing or handling them explicitly.
191-210: Consider validating external stream data with Zod.Multiple type assertions (
as) are used to handle incoming stream events. Since this is external data from the durable stream, using Zod schemas would provide runtime validation and better type safety.🔧 Example validation approach
import { z } from "zod"; const UserInputChunkSchema = z.object({ type: z.literal("chunk"), key: z.string(), value: z.object({ type: z.literal("user_input"), content: z.string(), }), }); // Then in the callback: const parsed = UserInputChunkSchema.safeParse(event); if (!parsed.success) continue; const { key, value } = parsed.data;
396-396: Extract model name to a named constant.The model name
"claude-sonnet-4-5-20250929"is a magic string that should be extracted to a constant at module top for easier maintenance and visibility.🔧 Suggested fix
+const DEFAULT_CLAUDE_MODEL = "claude-sonnet-4-5-20250929"; + // ... at line 396: - model: process.env.CLAUDE_MODEL || "claude-sonnet-4-5-20250929", + model: process.env.CLAUDE_MODEL || DEFAULT_CLAUDE_MODEL,As per coding guidelines: Extract hardcoded magic numbers, strings, and enums to named constants at module top instead of leaving them inline in logic.
399-399: Add a comment explaining whybypassPermissionsis used in desktop context.The
permissionMode: "bypassPermissions"is appropriate for the desktop app where Claude executes code locally on the user's own machine. However, add a comment explaining this is intentional for the desktop context to clarify the security rationale for future maintainers.// In a desktop context, users run Claude on their own machine, so bypassing // permission prompts for local code execution is appropriate. permissionMode: "bypassPermissions" as const,
| function getCredentialsFromKeychain(): ClaudeCredentials | null { | ||
| if (platform() !== "darwin") { | ||
| return null; | ||
| } | ||
|
|
||
| try { | ||
| // Claude CLI stores credentials in the keychain with this service/account | ||
| const result = execSync( | ||
| 'security find-generic-password -s "claude-cli" -a "api-key" -w 2>/dev/null', | ||
| { encoding: "utf-8" }, | ||
| ).trim(); | ||
|
|
||
| if (result) { | ||
| console.log("[claude/auth] Found credentials in macOS Keychain"); | ||
| return { apiKey: result, source: "keychain", kind: "apiKey" }; | ||
| } | ||
| } catch { | ||
| // Not found in keychain, this is fine | ||
| } | ||
|
|
||
| // Try alternate keychain entry format | ||
| try { | ||
| const result = execSync( | ||
| 'security find-generic-password -s "anthropic-api-key" -w 2>/dev/null', | ||
| { encoding: "utf-8" }, | ||
| ).trim(); | ||
|
|
||
| if (result) { | ||
| console.log( | ||
| "[claude/auth] Found credentials in macOS Keychain (anthropic-api-key)", | ||
| ); | ||
| return { apiKey: result, source: "keychain", kind: "apiKey" }; | ||
| } | ||
| } catch { | ||
| // Not found in keychain, this is fine | ||
| } | ||
|
|
||
| return null; | ||
| } |
There was a problem hiding this comment.
Don’t swallow credential lookup/parse errors silently.
Add a prefixed log so unexpected failures are diagnosable.
🛠️ Suggested fix
- } catch {
- // Not found in keychain, this is fine
- }
+ } catch (error) {
+ console.debug(
+ "[claude/auth] Keychain lookup for claude-cli failed (continuing):",
+ error,
+ );
+ }
@@
- } catch {
- // Not found in keychain, this is fine
- }
+ } catch (error) {
+ console.debug(
+ "[claude/auth] Keychain lookup for anthropic-api-key failed (continuing):",
+ error,
+ );
+ }
@@
- } catch {
- return false;
- }
+ } catch (error) {
+ console.debug(
+ "[claude/auth] Failed to parse OAuth credentials (continuing):",
+ error,
+ );
+ return false;
+ }Also applies to: 237-254
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/utils/auth/auth.ts` around lines
112 - 150, In getCredentialsFromKeychain, replace the empty catch blocks with
catches that log the unexpected error with a clear prefix and the error details
(e.g., catch (err) { console.error("[claude/auth] keychain lookup error:", err);
}) so failures during credential lookup/parse are diagnosable; do the same for
the second keychain attempt inside getCredentialsFromKeychain and apply the
identical logging change to the other credential lookup block referenced around
lines 237-254 so no credential-related errors are swallowed silently.
| onError: (error) => { | ||
| console.warn( | ||
| `[stream-watcher] Stream error for ${this.sessionId}:`, | ||
| error, | ||
| ); | ||
| return {}; | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager && cat -n session-manager.ts | sed -n '160,190p'Repository: superset-sh/superset
Length of output: 975
🏁 Script executed:
rg "@durable-streams" --type ts --type tsx -A 2 -B 2Repository: superset-sh/superset
Length of output: 91
🌐 Web query:
@durable-streams/client stream onError callback return type documentation
💡 Result:
I couldn’t find any official, public documentation that states the return type of stream({ onError }) in @durable-streams/client. The only public docs I found that mention the JS client (@durable-streams/client) show basic stream({ ... }) usage but don’t document an onError option at all. [1]
What you can do instead (authoritative, because it’s the package’s own types):
- Inspect the TypeScript declaration shipped with the package:
node_modules/@durable-streams/client/dist/index.d.ts(or similar), and look for theStreamOptions/stream()options type.- That will tell you the exact callback signature, including the
onErrorreturn type.
If you want, paste the relevant StreamOptions / onError type from that .d.ts file here and I’ll interpret it.
Sources:
[1] AnyCable Durable Streams docs (shows @durable-streams/client usage but no onError typing/return docs): (docs.anycable.io)
Citations:
🏁 Script executed:
rg "package.json" --type json | head -1 | xargs cat | jq '.dependencies, .devDependencies | select(. != null) | .["@durable-streams/client"]'Repository: superset-sh/superset
Length of output: 245
🏁 Script executed:
find . -name "*.d.ts" -type f | xargs rg -l "DurableStream|stream.*onError" 2>/dev/null | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
rg "onError" apps/desktop/src --type ts -B 3 -A 3Repository: superset-sh/superset
Length of output: 50377
🏁 Script executed:
rg "DurableStream" apps/desktop/src --type ts -B 2 -A 10 | head -60Repository: superset-sh/superset
Length of output: 6652
🏁 Script executed:
find . -type f -name "package.json" -path "*/apps/desktop/*" -exec grep -l "@durable-streams" {} \;Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
grep -r "@durable-streams/client" --include="package.json" | head -3Repository: superset-sh/superset
Length of output: 197
🏁 Script executed:
rg "new DurableStream" apps/desktop/src --type ts -A 15 | head -80Repository: superset-sh/superset
Length of output: 1844
🏁 Script executed:
# Check if we can find type definitions for `@durable-streams/client`
find node_modules/@durable-streams/client -name "*.d.ts" 2>/dev/null | head -5Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Try to read the type definitions if they exist
cat node_modules/@durable-streams/client/dist/index.d.ts 2>/dev/null | rg -A 15 "stream\(" | head -60Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there are other stream() usages in the codebase to see the pattern
rg "handle\.stream\(" apps/desktop/src --type ts -A 20 | head -80Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Search for stream implementations that might have similar patterns
rg "\.stream\(" --type ts -A 12 | grep -A 12 "onError"Repository: superset-sh/superset
Length of output: 46
🏁 Script executed:
# Check if there are comments or documentation about onError in the codebase
rg "onError.*return" --type ts -B 3 -A 3Repository: superset-sh/superset
Length of output: 2384
🏁 Script executed:
# Look at the full context of the StreamWatcher class to understand the intent
sed -n '160,230p' apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.tsRepository: superset-sh/superset
Length of output: 1715
Clarify the purpose of the empty object return in the onError callback or remove it.
The onError callback returns an empty object {}, which differs from typical error handler patterns in the codebase. Either document why this return value is needed (if required by the @durable-streams/client API), or remove it if it serves no purpose.
🤖 Prompt for AI Agents
In
`@apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-manager.ts`
around lines 172 - 178, The onError callback in the stream watcher (the onError
handler that logs `[stream-watcher] Stream error for ${this.sessionId}`)
currently returns an empty object `{}`; either remove that return so the handler
returns void/undefined, or if the `@durable-streams/client` API requires a
specific return value, replace the `{}` with the correct typed return and add a
short comment explaining why that value is required; update the onError in
session-manager.ts (the stream watcher callback) and include a one-line comment
referencing `@durable-streams/client` behavior if you keep a non-void return.
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/ai-chat-plan.md (1)
306-316:⚠️ Potential issue | 🟡 MinorAdd a language to the fenced block to satisfy MD040.
Markdownlint expects a language identifier on fenced blocks.
✍️ Suggested diff
-``` +```text apps/streams/ ├── package.json ├── Dockerfile ├── fly.toml └── src/ ├── index.ts # Hono server ├── streams.ts # Durable Stream handlers └── presence.ts # State Protocol handlers</details> </blockquote></details> </blockquote></details>🤖 Fix all issues with AI agents
In @.env.example: - Around line 68-72: Swap the two environment variable entries so DURABLE_STREAM_AUTH_TOKEN appears before DURABLE_STREAM_URL to satisfy dotenv-linter ordering; update the .env.example file by moving the DURABLE_STREAM_AUTH_TOKEN line above the DURABLE_STREAM_URL line (keep both keys and their empty values unchanged) so the file lists DURABLE_STREAM_AUTH_TOKEN then DURABLE_STREAM_URL.
| # ----------------------------------------------------------------------------- | ||
| # Durable Streams | ||
| # ----------------------------------------------------------------------------- | ||
| DURABLE_STREAM_URL= | ||
| DURABLE_STREAM_AUTH_TOKEN= |
There was a problem hiding this comment.
Swap key order to satisfy dotenv-linter.
The linter expects DURABLE_STREAM_AUTH_TOKEN before DURABLE_STREAM_URL. Consider swapping to avoid warnings.
♻️ Suggested diff
-DURABLE_STREAM_URL=
-DURABLE_STREAM_AUTH_TOKEN=
+DURABLE_STREAM_AUTH_TOKEN=
+DURABLE_STREAM_URL=📝 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.
| # ----------------------------------------------------------------------------- | |
| # Durable Streams | |
| # ----------------------------------------------------------------------------- | |
| DURABLE_STREAM_URL= | |
| DURABLE_STREAM_AUTH_TOKEN= | |
| # ----------------------------------------------------------------------------- | |
| # Durable Streams | |
| # ----------------------------------------------------------------------------- | |
| DURABLE_STREAM_AUTH_TOKEN= | |
| DURABLE_STREAM_URL= |
🧰 Tools
🪛 dotenv-linter (4.0.0)
[warning] 72-72: [UnorderedKey] The DURABLE_STREAM_AUTH_TOKEN key should go before the DURABLE_STREAM_URL key
(UnorderedKey)
🤖 Prompt for AI Agents
In @.env.example around lines 68 - 72, Swap the two environment variable entries
so DURABLE_STREAM_AUTH_TOKEN appears before DURABLE_STREAM_URL to satisfy
dotenv-linter ordering; update the .env.example file by moving the
DURABLE_STREAM_AUTH_TOKEN line above the DURABLE_STREAM_URL line (keep both keys
and their empty values unchanged) so the file lists DURABLE_STREAM_AUTH_TOKEN
then DURABLE_STREAM_URL.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@docs/ai-chat-plan.md`:
- Around line 373-415: The schema for chatSessions, chatMessages and
chatParticipants lacks database indexes needed for production queries; add
appropriate indexes after the table definitions by creating index objects that
target the suggested columns (e.g., for chatSessions add an index on
organizationId + createdAt DESC and on repositoryId + archivedAt and on
createdById + createdAt DESC; for chatMessages add indexes on sessionId +
createdAt, organizationId + createdAt DESC, and on processingStartedAt +
processingExpiresAt; for chatParticipants add indexes on sessionId + userId and
userId + joinedAt DESC) using the same index factory used elsewhere (name them
uniquely, e.g., chat_sessions_org_created_at_idx,
chat_messages_session_created_at_idx, chat_participants_session_user_idx) so
queries like listing sessions, fetching ordered messages, and membership checks
are covered.
- Around line 271-314: The PR currently keeps session mapping in an in-memory
Map<sessionId, claudeSessionId> and accepts tool_result → tool-result chunks
without security controls; fix by (1) replacing the in-memory Map in
apps/streams/src/claude-agent.ts with a pluggable SessionStore interface
(methods: get(sessionId), set(sessionId, claudeSessionId), delete(sessionId))
and provide at least two implementations (Redis-backed and optional file/SQLite
fallback) wired via env config (e.g., SESSION_STORE_TYPE) so multi-turn resumes
survive restarts and multiple instances and (2) harden sdk-to-ai-chunks
conversion for tool_result/tool-call outputs by adding explicit allowlist of
safe tool types, validating and sanitizing tool result payloads before producing
StreamChunk type "tool-result" or "tool-call", accumulating JSON safely,
stripping/normalizing file paths, redacting secrets/PII patterns, and adding
per-session rate limiting hooks (token bucket) and logging/metrics; also add a
short "Session Persistence" and "Security Considerations" section in
docs/ai-chat-plan.md describing the chosen persistence options, configuration,
validation rules, and where to find the SessionStore implementations and
sdk-to-ai-chunks sanitizers.
- Around line 162-179: The docs declare zod "^4.1.12" inside the vendored
`@superset/durable-session` dependency block but the monorepo already has zod
"^4.3.5", creating a breaking-version mismatch; update the vendored dependency
declaration for `@superset/durable-session` to use zod "^4.3.5" (or explicitly
document why 4.1.12 is required) and run/mention a quick compatibility check for
code paths using Zod methods that changed in v4.3.x (notably .pick(), .omit() on
refined object schemas and removal of .refine() from ZodMiniType) to ensure no
runtime incompatibility with functions or modules that rely on those behaviors.
🧹 Nitpick comments (6)
docs/ai-chat-plan.md (6)
5-65: Consider documenting security and resilience patterns in the architecture.The architecture diagram clearly shows the component flow, but several operational concerns are not addressed:
- Authentication between proxy and agent: The proxy calls the Claude agent endpoint via HTTP, but there's no mention of authentication/authorization for these internal calls.
- Network resilience: No timeout configuration or retry logic documented for the proxy → agent communication.
- Error propagation: How do agent failures surface to the client? Is there a structured error response format?
- Rate limiting: No mention of rate limiting at the proxy layer to protect the Claude agent endpoint.
Consider adding a subsection on security boundaries and error handling patterns.
67-78: Document error handling and idempotency guarantees in the message flow.The message flow describes the happy path, but several edge cases need clarification:
- Optimistic rollback: If step 5 (agent invocation) fails, what happens to the optimistic insert from step 2? Does the client need to remove it, or does the proxy send a failure event?
- Message ID coordination: How is the client-generated message ID (from optimistic insert) reconciled with the server-assigned ID?
- Idempotency: If the POST to
/v1/sessions/:id/messagesis retried due to network issues, how do you prevent duplicate messages in the durable stream?Consider adding a subsection on error recovery and idempotency guarantees.
79-87: Document the vendoring maintenance strategy.Decision
#1mentions vendoring ~4500 LOC fromelectric-sql/transport, but there's no documentation on:
- Update cadence: How often will you sync with upstream changes?
- Customization tracking: How will you track local modifications vs. upstream code?
- Security patches: If the upstream repository publishes a security fix, what's the process for applying it?
- Divergence risk: Over time, vendored code tends to diverge from upstream, making updates harder.
Consider adding a "Vendoring Strategy" section documenting:
- A
VENDOR.mdfile tracking the source commit SHA and local patches- A quarterly review process for upstream updates
- A clear policy on when to contribute fixes back upstream vs. maintaining local patches
106-126: Acknowledge the v1 → v2 transition risk.The status table shows several v1 components marked as DONE (custom DurableChatClient, custom materialization, custom stream server) but Phase C explicitly removes these files in favor of the vendored
@superset/durable-sessionpackage.This "build then replace" approach has tradeoffs:
- ✅ Benefit: v1 provides working reference implementation for validating vendored code
- ❌ Risk: Effort spent on v1 code that will be deleted
- ❌ Risk: Potential for incomplete migration if some v1 logic isn't covered by vendored code
Consider documenting:
- Which v1 patterns/features must be validated in the vendored code before removal
- A feature parity checklist for the v1 → v2 migration
- Whether v1 code should be preserved in a branch for reference
365-369: Document the behavioral change for drafts.Phase C3 changes drafts from stream-backed (synchronized across clients) to local React state (device-only). This is a significant UX change:
Before:
- ✅ Draft content syncs across devices via the
draftscollection- ✅ If a user switches devices, their draft is preserved
After:
- ❌ Draft content is local-only (Zustand or React state)
- ❌ Switching devices loses the draft
- ✅ Simpler implementation (no stream writes)
Typing indicators:
The note mentions usingpresence.status: 'typing'for indicators, but:
- Is this a custom extension to the vendored schema?
- How frequently are presence updates sent? (Typing events can be high-frequency)
Recommendation: Add a "UX Changes" section documenting:
- Drafts are no longer synchronized across devices
- Presence-based typing indicators update frequency and debouncing strategy
7-7: Add language specifiers to fenced code blocks.The markdown linter flags five fenced code blocks without language specifiers. While these are directory tree structures (not executable code), specifying a language improves rendering and accessibility.
📝 Recommended fix
Change:
``` directory structure here ```To:
```text directory structure here ```Apply to lines: 7, 137, 227, 430, 474.
This is a minor documentation quality improvement.
Also applies to: 137-137, 227-227, 430-430, 474-474
| **Dependencies** (all published on npm): | ||
| ```json | ||
| { | ||
| "name": "@superset/durable-session", | ||
| "dependencies": { | ||
| "@durable-streams/state": "^0.2.0", | ||
| "@standard-schema/spec": "^1.0.0", | ||
| "@tanstack/ai": "^0.3.0", | ||
| "@tanstack/db": "^0.5.22", | ||
| "@tanstack/db-ivm": "^0.1.17", | ||
| "zod": "^4.1.12" | ||
| }, | ||
| "peerDependencies": { | ||
| "react": "^18.0.0 || ^19.0.0", | ||
| "@tanstack/react-db": "^0.1.66" | ||
| } | ||
| } | ||
| ``` |
There was a problem hiding this comment.
❓ Verification inconclusive
Script executed:
# First, check if the file exists and examine the specified lines
wc -l docs/ai-chat-plan.mdRepository: superset-sh/superset
Repository: superset-sh/superset
Exit code: 0
stdout:
1190 docs/ai-chat-plan.md
Script executed:
# Check lines 162-179 (the snippet context shown in the review)
sed -n '162,179p' docs/ai-chat-plan.mdRepository: superset-sh/superset
Repository: superset-sh/superset
Exit code: 0
stdout:
**Specific import changes in react files:**
```typescript
// BEFORE (in react-durable-session source):
import { DurableChatClient, messageRowToUIMessage } from '@electric-sql/durable-session'
import type { DurableChatClientOptions } from '@electric-sql/durable-session'
// AFTER (in packages/durable-session/src/react/):
import { DurableChatClient, messageRowToUIMessage } from '..'
import type { DurableChatClientOptions } from '..'
```
```typescript
// BEFORE (react index.ts re-exports):
export { DurableChatClient, ... } from '@electric-sql/durable-session'
// AFTER:
export { DurableChatClient, ... } from '..'
Script executed:
# Check lines around 507 (where the other zod version is claimed)
sed -n '500,515p' docs/ai-chat-plan.mdRepository: superset-sh/superset
Repository: superset-sh/superset
Exit code: 0
stdout:
### B2. Create `apps/streams/src/sdk-to-ai-chunks.ts`
Pure conversion module. Maps Claude SDK `SDKMessage` types to TanStack AI `StreamChunk`.
**The proxy expects standard JSON chunks** — it reads SSE `data: {...}` lines, parses JSON, and writes each chunk to the durable stream via `protocol.writeChunk()`. The `StreamProcessor` on the client side then materializes these into `MessagePart[]`.
#### Conversion Table
| SDKMessage | TanStack AI Chunk | Notes |
|---|---|---|
| `stream_event` → `content_block_start` (text) | — | No chunk, wait for deltas |
| `stream_event` → `content_block_delta` (text_delta) | `{ type: "text-delta", textDelta }` | |
| `stream_event` → `content_block_start` (tool_use) | `{ type: "tool-call-streaming-start", toolCallId, toolName }` | |
| `stream_event` → `content_block_delta` (input_json_delta) | `{ type: "tool-call-delta", toolCallId, argsTextDelta }` | |
| `stream_event` → `content_block_stop` (tool_use) | `{ type: "tool-call", toolCallId, toolName, args }` | Full args from accumulator |
Script executed:
# Search for all zod version mentions in the file
rg 'zod.*[0-9]+\.[0-9]+\.[0-9]+' docs/ai-chat-plan.md -nRepository: superset-sh/superset
Repository: superset-sh/superset
Exit code: 0
stdout:
207: "zod": "^4.1.12"
390: "zod": "^4.1.12"
847:- `zod` ^4.3.5
breaking changes zod 4.1.12 4.3.5 changelog
Between zod v4.1.12 (2025-10-06) and v4.3.5 (2026-01-04), the notable breaking / behavior-changing items called out in the release notes are:
- v4.3.0:
.pick()and.omit()now throw when used on object schemas that contain refinements. Previously, Zod would silently drop the refinements (unsound), so code that relied on that will break. The release notes include a migration suggestion: rebuild a new object schema from the original schema’sshape. [1] - v4.3.4 (Zod Mini):
.refine()was removed fromZodMiniType, so any code using.refine()viazod/mini(or APIs relying onZodMiniType.refine) will break. [1]
Not a breaking change, but relevant when scanning “changelog”-type updates:
- v4.3.5: docs were updated for the deprecation of the
messageparameter (i.e., it’s a deprecation notice, not an immediate breaking change). [1]
Source: Zod GitHub releases page (v4.1.12 → v4.3.5). [1]
Resolve the zod version conflict across the documentation.
The document specifies zod: "^4.1.12" in the vendored @superset/durable-session dependencies (lines 207, 390), but line 847 indicates zod ^4.3.5 is already installed in the monorepo. There are actual breaking changes between these versions:
- v4.3.0:
.pick()and.omit()now throw on object schemas containing refinements (previously silent, unsound behavior) - v4.3.4:
.refine()was removed fromZodMiniType
If the vendored package or any code relies on these modified behaviors, incompatibility will surface at runtime. Align the versions: either upgrade the vendored dependency to ^4.3.5 or document why 4.1.12 is required.
🤖 Prompt for AI Agents
In `@docs/ai-chat-plan.md` around lines 162 - 179, The docs declare zod "^4.1.12"
inside the vendored `@superset/durable-session` dependency block but the monorepo
already has zod "^4.3.5", creating a breaking-version mismatch; update the
vendored dependency declaration for `@superset/durable-session` to use zod
"^4.3.5" (or explicitly document why 4.1.12 is required) and run/mention a quick
compatibility check for code paths using Zod methods that changed in v4.3.x
(notably .pick(), .omit() on refined object schemas and removal of .refine()
from ZodMiniType) to ensure no runtime incompatibility with functions or modules
that rely on those behaviors.
| ## Phase B: Claude Agent Endpoint | ||
|
|
||
| ### Files to Create | ||
| ### B1. Create `apps/streams/src/claude-agent.ts` | ||
|
|
||
| **`apps/desktop/src/main/lib/claude/session-manager.ts`** (new) | ||
| - Manages Claude SDK sessions | ||
| - Spawns bundled binary with user's auth | ||
| - Streams tokens to Durable Stream | ||
| - Emits events for local tRPC subscription | ||
| HTTP endpoint the proxy invokes when a user sends a message: | ||
|
|
||
| **`apps/desktop/src/main/lib/trpc/routers/ai-chat.ts`** (new) | ||
| ```typescript | ||
| export const aiChatRouter = router({ | ||
| // Start or resume a session | ||
| startSession: publicProcedure | ||
| .input(z.object({ | ||
| sessionId: z.string(), | ||
| cwd: z.string(), | ||
| claudeSessionId: z.string().optional(), | ||
| })) | ||
| .mutation(async ({ input }) => { ... }), | ||
|
|
||
| // Send message and trigger Claude | ||
| sendMessage: publicProcedure | ||
| .input(z.object({ | ||
| sessionId: z.string(), | ||
| content: z.string(), | ||
| })) | ||
| .mutation(async ({ input }) => { ... }), | ||
|
|
||
| // Local token stream (observable pattern per AGENTS.md) | ||
| streamTokens: publicProcedure | ||
| .input(z.object({ sessionId: z.string() })) | ||
| .subscription(({ input }) => { | ||
| return observable<StreamEvent>((emit) => { | ||
| // Subscribe to session manager events | ||
| return () => { /* cleanup */ }; | ||
| }); | ||
| }), | ||
|
|
||
| // Interrupt generation | ||
| interrupt: publicProcedure | ||
| .input(z.object({ sessionId: z.string() })) | ||
| .mutation(async ({ input }) => { ... }), | ||
| }); | ||
| // Hono app with POST / | ||
| // 1. Receives { messages: Array<{ role, content, parts }> } from proxy | ||
| // 2. Extracts latest user message as prompt | ||
| // 3. Runs query() from @anthropic-ai/claude-agent-sdk | ||
| // 4. Converts SDKMessage stream → TanStack AI SSE chunks | ||
| // 5. Returns SSE Response | ||
| ``` | ||
|
|
||
| **`apps/desktop/src/main/lib/trpc/routers/index.ts`** - Add aiChatRouter | ||
|
|
||
| --- | ||
| **Session state:** Maintains `Map<sessionId, claudeSessionId>` for multi-turn resume. | ||
|
|
||
| ## Phase 4: API Session Trigger | ||
| **Binary path:** From `CLAUDE_BINARY_PATH` env var (set by desktop app when starting streams process). | ||
|
|
||
| The API handles session creation and message sending. When a client (web/mobile) wants to start a chat: | ||
| **Auth:** From `CLAUDE_AUTH_*` env vars forwarded from desktop process. | ||
|
|
||
| 1. Client calls API `chat.createSession` or `chat.sendMessage` | ||
| 2. API writes session/message request to Postgres | ||
| 3. Desktop app subscribed to workspace picks up via Electric subscription | ||
| 4. Desktop runs Claude SDK and streams to Durable Stream | ||
| 5. All clients see tokens via Durable Stream subscription | ||
| ### B2. Create `apps/streams/src/sdk-to-ai-chunks.ts` | ||
|
|
||
| **API Router (in `packages/trpc/src/router/chat/index.ts`):** | ||
| ```typescript | ||
| export const chatRouter = createTRPCRouter({ | ||
| // Create a new chat session | ||
| createSession: protectedProcedure | ||
| .input(z.object({ | ||
| repositoryId: z.string().uuid().optional(), | ||
| workspaceId: z.string().optional(), | ||
| title: z.string().optional(), | ||
| })) | ||
| .mutation(async ({ ctx, input }) => { | ||
| // Insert into chat_sessions table | ||
| // Returns sessionId | ||
| }), | ||
|
|
||
| // Send a message (triggers Claude on desktop) | ||
| sendMessage: protectedProcedure | ||
| .input(z.object({ | ||
| sessionId: z.string().uuid(), | ||
| content: z.string(), | ||
| })) | ||
| .mutation(async ({ ctx, input }) => { | ||
| // Insert user message into chat_messages | ||
| // Desktop picks this up and runs Claude | ||
| }), | ||
|
|
||
| // ... other CRUD operations | ||
| }); | ||
| ``` | ||
| Pure conversion module. Maps Claude SDK `SDKMessage` types to TanStack AI `StreamChunk`: | ||
|
|
||
| **Desktop subscription:** | ||
| ```typescript | ||
| // Subscribe to pending messages for workspaces user has open | ||
| const pendingMessages = electricClient.stream({ | ||
| shape: { | ||
| table: "chat_messages", | ||
| where: `session_id IN (...) AND role = 'user' AND NOT processed`, | ||
| } | ||
| }); | ||
| | SDKMessage | TanStack AI Chunk | Notes | | ||
| |---|---|---| | ||
| | `stream_event` → `content_block_start` (text) | — | No chunk, wait for deltas | | ||
| | `stream_event` → `content_block_delta` (text_delta) | `{ type: "text-delta", textDelta }` | | | ||
| | `stream_event` → `content_block_start` (tool_use) | `{ type: "tool-call-streaming-start", toolCallId, toolName }` | | | ||
| | `stream_event` → `content_block_delta` (input_json_delta) | `{ type: "tool-call-delta", toolCallId, argsTextDelta }` | | | ||
| | `stream_event` → `content_block_stop` (tool_use) | `{ type: "tool-call", toolCallId, toolName, args }` | Full args from accumulator | | ||
| | `stream_event` → `content_block_start` (thinking) | — | Wait for deltas | | ||
| | `stream_event` → `content_block_delta` (thinking_delta) | `{ type: "reasoning", textDelta }` | | | ||
| | `user` (tool_result blocks) | `{ type: "tool-result", toolCallId, result }` | Server-side tool execution | | ||
| | `result` | `{ type: "finish", finishReason: "stop" }` | End of agent turn | | ||
| | `system` (init) | — | Extract `claudeSessionId` internally | | ||
| | `assistant` | — | Skip (stream_events already cover content) | | ||
|
|
||
| pendingMessages.subscribe((messages) => { | ||
| for (const msg of messages) { | ||
| runClaudeSession(msg); | ||
| } | ||
| }); | ||
| ``` | ||
| **ConversionState** tracks: | ||
| - Active content block indices (to correlate starts with deltas) | ||
| - JSON accumulator per tool_use block (for partial → full args) | ||
| - Current tool call IDs per block index | ||
|
|
There was a problem hiding this comment.
Address session persistence and tool execution security.
Two concerns in the Claude Agent Endpoint design:
1. Session state persistence (Line 286):
The Map<sessionId, claudeSessionId> is in-memory only. While line 549 mentions "optional file-based persistence," there's no concrete implementation plan. Without persistence:
- Multi-turn conversations are lost on agent restart
- Load balancing across multiple agent instances is impossible
- Deployments require session draining
2. Tool execution security (Line 305):
The conversion table shows user (tool_result blocks) → tool-result chunk. If Claude SDK executes tools server-side and returns results, there are security risks:
- Path traversal: Tool results containing file paths could expose sensitive directories
- Data exfiltration: Tool results might contain secrets, credentials, or PII
- Injection: Malicious tool results could inject code if improperly sanitized before rendering
Recommendations:
- Document the session persistence implementation (Redis? SQLite? File-based JSON?)
- Add a "Security Considerations" section covering:
- Input validation for tool results
- Sanitization before writing to durable stream
- Allowlist of safe tool types
- Rate limiting per session
🤖 Prompt for AI Agents
In `@docs/ai-chat-plan.md` around lines 271 - 314, The PR currently keeps session
mapping in an in-memory Map<sessionId, claudeSessionId> and accepts tool_result
→ tool-result chunks without security controls; fix by (1) replacing the
in-memory Map in apps/streams/src/claude-agent.ts with a pluggable SessionStore
interface (methods: get(sessionId), set(sessionId, claudeSessionId),
delete(sessionId)) and provide at least two implementations (Redis-backed and
optional file/SQLite fallback) wired via env config (e.g., SESSION_STORE_TYPE)
so multi-turn resumes survive restarts and multiple instances and (2) harden
sdk-to-ai-chunks conversion for tool_result/tool-call outputs by adding explicit
allowlist of safe tool types, validating and sanitizing tool result payloads
before producing StreamChunk type "tool-result" or "tool-call", accumulating
JSON safely, stripping/normalizing file paths, redacting secrets/PII patterns,
and adding per-session rate limiting hooks (token bucket) and logging/metrics;
also add a short "Session Persistence" and "Security Considerations" section in
docs/ai-chat-plan.md describing the chosen persistence options, configuration,
validation rules, and where to find the SessionStore implementations and
sdk-to-ai-chunks sanitizers.
| ## Phase D: Database Schema | ||
|
|
||
| **`packages/ai-chat/src/hooks/useDurableStream.ts`** (new) | ||
| **`packages/db/src/schema/chat.ts`** (new): | ||
| ```typescript | ||
| export function useDurableStream(sessionId: string | null) { | ||
| // Subscribe to Durable Stream via SSE | ||
| // Track offset for resume | ||
| // Return: { streamingContent, isConnected, events } | ||
| } | ||
| ``` | ||
| export const chatSessions = pgTable("chat_sessions", { | ||
| id: uuid().primaryKey().defaultRandom(), | ||
| organizationId: uuid("organization_id").notNull().references(() => organizations.id), | ||
| repositoryId: uuid("repository_id").references(() => repositories.id), | ||
| workspaceId: text("workspace_id"), | ||
| title: text().notNull(), | ||
| claudeSessionId: text("claude_session_id"), | ||
| cwd: text(), | ||
| createdById: uuid("created_by_id").notNull().references(() => users.id), | ||
| archivedAt: timestamp("archived_at"), | ||
| createdAt: timestamp("created_at").notNull().defaultNow(), | ||
| updatedAt: timestamp("updated_at").notNull().defaultNow().$onUpdate(() => new Date()), | ||
| }); | ||
|
|
||
| **`packages/ai-chat/src/hooks/usePresence.ts`** (new) | ||
| ```typescript | ||
| export function usePresence(sessionId: string | null, user: User) { | ||
| // Poll/subscribe to presence | ||
| // Heartbeat for viewing status | ||
| // Return: { viewers, typingUsers, setTyping } | ||
| } | ||
| ``` | ||
| export const chatMessages = pgTable("chat_messages", { | ||
| id: uuid().primaryKey().defaultRandom(), | ||
| sessionId: uuid("session_id").notNull().references(() => chatSessions.id), | ||
| organizationId: uuid("organization_id").notNull().references(() => organizations.id), | ||
| role: text().notNull(), | ||
| content: text().notNull(), | ||
| toolCalls: jsonb("tool_calls"), | ||
| inputTokens: integer("input_tokens"), | ||
| outputTokens: integer("output_tokens"), | ||
| createdById: uuid("created_by_id").references(() => users.id), | ||
| processingStartedAt: timestamp("processing_started_at"), | ||
| processingExpiresAt: timestamp("processing_expires_at"), | ||
| processedAt: timestamp("processed_at"), | ||
| processingError: text("processing_error"), | ||
| createdAt: timestamp("created_at").notNull().defaultNow(), | ||
| }); | ||
|
|
||
| **`packages/ai-chat/src/hooks/useMultiplayerChat.ts`** (new) | ||
| ```typescript | ||
| export function useMultiplayerChat(sessionId: string) { | ||
| // Combine: | ||
| // - Electric SQL for completed messages | ||
| // - Durable Stream for live tokens | ||
| // - Presence for typing indicators | ||
| // Return: { messages, sendMessage, viewers, typingUsers, isStreaming } | ||
| } | ||
| export const chatParticipants = pgTable("chat_participants", { | ||
| id: uuid().primaryKey().defaultRandom(), | ||
| sessionId: uuid("session_id").notNull().references(() => chatSessions.id), | ||
| userId: uuid("user_id").notNull().references(() => users.id), | ||
| role: text().notNull().default("viewer"), | ||
| joinedAt: timestamp("joined_at").notNull().defaultNow(), | ||
| }); | ||
| ``` |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Define indexes for the chat schema.
The schema defines three tables but no indexes. Production queries will need indexes on:
chat_sessions:
(organization_id, created_at DESC)— List sessions for an org(repository_id, archived_at)— Active sessions for a repo(created_by_id, created_at DESC)— User's session history
chat_messages:
(session_id, created_at)— Ordered messages for a session (critical path)(organization_id, created_at DESC)— Org-wide message search(processing_started_at, processing_expires_at)— Find stuck/expired processing
chat_participants:
(session_id, user_id)— Check participant membership (auth)(user_id, joined_at DESC)— User's joined sessions
🔍 Proposed index additions
Add after the table definitions:
// Indexes
export const chatSessionsOrgCreatedAtIdx = index("chat_sessions_org_created_at_idx")
.on(chatSessions.organizationId, chatSessions.createdAt.desc());
export const chatMessagesSessionCreatedAtIdx = index("chat_messages_session_created_at_idx")
.on(chatMessages.sessionId, chatMessages.createdAt);
export const chatParticipantsSessionUserIdx = index("chat_participants_session_user_idx")
.on(chatParticipants.sessionId, chatParticipants.userId);This is essential for query performance as chat history grows.
🤖 Prompt for AI Agents
In `@docs/ai-chat-plan.md` around lines 373 - 415, The schema for chatSessions,
chatMessages and chatParticipants lacks database indexes needed for production
queries; add appropriate indexes after the table definitions by creating index
objects that target the suggested columns (e.g., for chatSessions add an index
on organizationId + createdAt DESC and on repositoryId + archivedAt and on
createdById + createdAt DESC; for chatMessages add indexes on sessionId +
createdAt, organizationId + createdAt DESC, and on processingStartedAt +
processingExpiresAt; for chatParticipants add indexes on sessionId + userId and
userId + joinedAt DESC) using the same index factory used elsewhere (name them
uniquely, e.g., chat_sessions_org_created_at_idx,
chat_messages_session_created_at_idx, chat_participants_session_user_idx) so
queries like listing sessions, fetching ordered messages, and membership checks
are covered.
1 participant
Summary
.lfsconfig@anthropic-ai/claude-agent-sdkdependencyContext
Split from
ai-chatbranch. Depends on PR #1 (shared-package) for types.Test plan
bun run download:claudefetches the binarySummary by CodeRabbit
New Features
Chores
Docs