superagent#1413
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughReworks desktop AI chat to a client-driven streaming model: adds a Mastra-based superagent runtime, per-session streaming/orchestration and approval flows, OAuth credential sourcing, new tRPC streaming/approval endpoints, message hydration/tool-part modeling, many UI components/hooks, and supporting agent tools and packages. Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/UI
participant ChatIF as ChatInterface
participant Hook as useSuperagentStream
participant tRPC as aiChat tRPC
participant Agent as Superagent/Agent
participant Store as Storage/Context
User->>ChatIF: send message (handleSend)
ChatIF->>tRPC: superagent mutation (session, input, context)
tRPC->>Store: fetch/inject credentials & context
tRPC->>Agent: start agent run
Agent->>tRPC: emit streaming chunks
tRPC->>Hook: forward chunks via superagentStream subscription
loop chunk processing
Hook->>Hook: parse chunk (text/tool/agent/usage/approval)
Hook->>ChatIF: update messages, parts, usage, pendingApproval
ChatIF->>User: render message parts / approval UI
end
alt tool requires approval
Agent->>tRPC: emit tool-call-approval
tRPC->>Hook: mark suspended/runId
Hook->>ChatIF: show ToolApprovalBar
User->>ChatIF: approve/decline
ChatIF->>tRPC: call approveToolCall
tRPC->>Agent: resume run
Agent->>tRPC: continue streaming
end
Agent->>tRPC: done
Hook->>ChatIF: finalize message, clear streaming
ChatIF->>User: display completed response
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 3❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (1 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 |
# Conflicts: # apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx # bun.lock
Extract types, constants, utilities, components, and hooks from the 1,537-line ChatInterface.tsx into 10 co-located files for maintainability. No behavioral changes.
🧹 Preview Cleanup CompleteThe following preview resources have been cleaned up:
Thank you for your contribution! 🎉 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 19
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 497-514: The sendMessage mutation currently does nothing (just
logs and returns { success: true }); either restore the original agent dispatch
by uncommenting and re-enabling the chatSessionManager.startAgent(...)
fire-and-forget call (including its .catch(...) error logging) and remove the
stray console.log, or delete the sendMessage procedure entirely if the frontend
now uses a different endpoint (e.g., superagent). Locate the sendMessage
publicProcedure and either re-enable the chatSessionManager.startAgent
invocation with proper error handling or remove the procedure to avoid a no-op
API surface.
- Around line 536-545: Concurrent requests for the same sessionId overwrite the
AbortController in sessionAbortControllers, leaving the previous stream
uncancellable; before creating and storing a new AbortController in the async
block, check sessionAbortControllers.get(input.sessionId) and if one exists call
.abort() on it and remove it, then create the new AbortController and set it
(sessionAbortControllers.set(input.sessionId, abortController)); ensure any
cleanup also updates sessionAbortControllers (and optionally sessionContext)
when streams finish or error to avoid stale controllers.
- Around line 663-679: The catch block for the superagent stream currently emits
an error but doesn't clean up sessionRunIds or sessionContext, causing stale
state; update the catch (error) handler so that if the error is not an
intentional abort (i.e., abortController.signal.aborted is false) you also
delete sessionRunIds.delete(input.sessionId) and
sessionContext.delete(input.sessionId) (the same keys you remove on successful
completion) before emitting the error event, ensuring sessionRunIds,
sessionContext, and sessionAbortControllers are all cleared when a stream fails.
- Around line 198-221: The parseFileMentions function currently uses join(cwd,
relPath) and can read files outside the project (path traversal); after
computing the candidate path (use path.resolve or normalize on join(cwd,
relPath)) verify that the resolved path remains inside cwd (for example by
ensuring path.relative(cwd, resolvedPath) does not start with '..' and is not
absolute outside cwd) and skip adding/reading any mention that escapes cwd;
update parseFileMentions to perform this check before calling safeReadFile so
only files under cwd are read.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`:
- Around line 173-191: handleApprove/handleDecline fire the mutation and clear
pendingApproval but never update the corresponding tool-call part in messages,
so the UI doesn't reflect the approval/denial; update the messages state
optimistically inside both handlers (before/alongside
approveToolCallMutation.mutate) by finding the message/part that matches
pendingApproval.runId and replacing its tool-call part status or metadata to
indicate approved=true or approved=false, using setMessages to return a new
messages array (immutably) so the UI shows the decision immediately; keep the
existing approveToolCallMutation.mutate call and sessionId/runId usage but
ensure the update targets the same runId and only modifies the tool-call part
(preserving other parts and message fields).
- Around line 79-84: The effect that sets local messages from historyMessages
(useEffect that calls setMessages(hydrateMessages(...))) overwrites in-progress
streaming content when history refetches; modify that effect to guard against
overwriting during an active stream (check an isStreaming flag or similar state
before calling setMessages) or alternatively disable unwanted refetching at the
source (set the tRPC query option refetchOnWindowFocus: false / disable polling)
so hydrateMessages is only applied when not streaming.
- Around line 219-223: The linter flags use of array index as key in the
msg.parts.map within ChatInterface; change the key to a stable composite key
(e.g., combine the parent message identifier and part index/type) instead of
key={i} so keys remain unique and stable across renders — locate the mapping
over msg.parts in the ChatInterface component (the span rendering of part.text)
and replace the index key with something like `${msg.id}-${part.type}-${i}` or
another unique identifier from the message.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/AgentCallBlock/AgentCallBlock.tsx`:
- Line 48: The ChevronDownIcon rotation isn't working because its utility
depends on a parent with the "group" class; update the CollapsibleTrigger
component (the JSX element named CollapsibleTrigger where className is set) to
include "group" in its className (e.g., add the token "group" alongside existing
classes like "flex w-full items-center justify-between gap-4 p-3") so the
group-data-[state=open]:rotate-180 on ChevronDownIcon takes effect.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx`:
- Around line 91-137: The switch-to-display mapping and filename-extraction
logic in MessagePartsRenderer (inside the exploringItems map using getArgs(p)
and p.toolName) is duplicated in ReadOnlyToolCall; extract a shared utility
function named getToolDisplayInfo(toolName, args, status) that returns {title,
subtitle, icon, isPending, isError} and centralize the filename-shortening logic
(e.g., take last path segment if subtitle includes "/") inside it, then replace
the switch and post-processing in MessagePartsRenderer and ReadOnlyToolCall to
call getToolDisplayInfo(getArgs(p).toolName? no — call
getToolDisplayInfo(p.toolName, args, p.status) and use its returned fields;
ensure produced titles for mastra_workspace_list_files are consistent
("Listing"/"Listed") and preserve existing icon choices (FileIcon,
FolderTreeIcon, FileSearchIcon, SearchIcon) and error/pending semantics.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ModelPicker/ModelPicker.tsx`:
- Line 42: The ModelPicker is calling electronTrpc.aiChat.getModels.useQuery but
ignores isLoading and error, leaving the selector blank on load/failure; update
the hook call to destructure isLoading and error alongside data (e.g., const {
data: models, isLoading, error } = electronTrpc.aiChat.getModels.useQuery()),
then render a loading option or spinner and disable the select when isLoading,
and render a clear error message or fallback option when error is present
(inside the ModelPicker component) so users see progress or failure instead of
an empty selector.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/constants.ts`:
- Around line 17-25: The READ_ONLY_TOOLS Set in constants.ts incorrectly
includes mutating entries mastra_workspace_mkdir and mastra_workspace_delete;
either remove those two entries so mkdir and delete go through the existing
approval flow (ToolApprovalBar) or, if the intent is to auto-approve them,
rename READ_ONLY_TOOLS to a clearer name like INLINE_TOOLS or
AUTO_APPROVED_TOOLS and update usages (e.g., ReadOnlyToolCall consumers) so
semantics match; update all references to READ_ONLY_TOOLS/ReadOnlyToolCall or
ToolApprovalBar accordingly to maintain correct UI behavior.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSuperagentStream/useSuperagentStream.ts`:
- Around line 616-644: The debug console.log inside the "tool-call-approval"
branch is leaking approval payloads; remove or guard the line that logs
JSON.stringify(chunk). Locate the switch case for "tool-call-approval" in
useSuperagentStream (the block that references runIdRef.current and calls
setPendingApproval) and either delete the console.log(...) call or wrap it
behind a development-only flag (e.g., process.env.NODE_ENV === "development") so
only non-production builds emit the chunk contents; keep the rest of the
approval handling (approvalData construction, setPendingApproval, and the
runIdRef.current check) unchanged.
- Around line 433-464: handleTopLevelChunk contains several comparisons between
existing tool-call parts and chunkToolCallId using raw equality which can fail
if chunkToolCallId is non-string; update every comparison in handleTopLevelChunk
(e.g., the parts.find and parts.map checks inside updateLastAssistant and any
other checks that compare pt.toolCallId to chunkToolCallId) to coerce
chunkToolCallId with String(chunkToolCallId) (or coerce pt.toolCallId
consistently) before comparing so the equality checks match the string IDs
(ensure you update all instances referenced in this diff such as the find at the
top and the map branch that builds or updates the "tool-call" part).
- Around line 193-208: The non-null assertions p.text! should be removed: after
the guard if (!p?.text) break; extract the value into a local const (e.g., const
text = p.text) and use that variable when creating/updating parts (inside the
updateActiveAgentParts callback where you currently reference p.text!). Apply
the same change in the other "text-delta" handler (the top-level handler around
the second occurrence) so both uses avoid the non-null assertion; reference the
parameter p and the updateActiveAgentParts call to locate the spots.
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/utils/hydrate-messages.ts`:
- Around line 10-32: The debug console.log in hydrate-messages.ts is leaking raw
message content; remove the console.log call that prints historyMessages (the
JSON.stringify block referencing historyMessages.slice(...)) or wrap it behind a
proper debug flag/logger check (e.g., only log when DEBUG or a dev-only logger
is enabled). Locate the console.log in the hydrate messages utility and either
delete it or replace it with a conditional debug-level log using your app's
logger (so production builds never print raw message content).
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/utils/tool-helpers.ts`:
- Around line 4-10: Handle the edge case where a ToolCallPart has status ===
"done" but no result and no isError: update toToolDisplayState to check for
part.status === "done" (before the final fallthrough) and return
"output-available" for a completed-but-empty-result call; mirror the same logic
in toWsToolState so both mappings treat a finished-but-empty tool call as an
output-available state rather than "input-available".
In `@packages/agent/src/index.ts`:
- Line 15: index.ts re-exporting "./superagent" causes PostgresStore to be
instantiated at module import, forcing DATABASE_URL/DB connection for all
consumers; remove the direct re-export from packages/agent/src/index.ts and
either (A) expose superagent via a separate entrypoint (e.g., create a dedicated
export path "@superset/agent/superagent" that re-exports the module) or (B)
change superagent.ts to lazy-initialize PostgresStore (provide a
getSuperagent/getStore factory function that constructs PostgresStore on first
use instead of at top-level). Update any callers to import the new entrypoint or
call the factory (referencing ./superagent, PostgresStore, and the current
top-level exports like createPermissionRequest/getActiveSessionCount to ensure
they no longer trigger DB initialization).
In `@packages/agent/src/superagent.ts`:
- Around line 11-14: The module currently uses a non-null assertion on
process.env.DATABASE_URL when constructing storage with new PostgresStore, which
the linter flags and which can produce unclear runtime failures at module load;
update the module to explicitly validate that process.env.DATABASE_URL is set
before creating storage (e.g., read process.env.DATABASE_URL into a local
variable, if falsy throw a clear Error like "DATABASE_URL is required" or return
a guarded factory), and then pass that validated string into new PostgresStore
so storage is only constructed with a guaranteed connection string.
In `@plan.md`:
- Around line 46-51: The current approach mutates process.env (e.g., setting
process.env.ANTHROPIC_API_KEY or process.env.OPENAI_API_KEY) around calls in the
superagent mutation (before superagent.stream() at the code referenced around
line 522) and the approveToolCall resumption path, which creates a race;
instead, refactor so you do NOT write to process.env: retrieve the provider
token from the credential store, then pass that token directly into the Mastra
agent/model constructor or into superagent.stream()/request options (or whatever
per-request auth API the Mastra/agent client exposes) for the specific call (use
the parsed provider from modelId such as "anthropic" to pick token), and remove
the set/restore of the env var; update both the superagent mutation and the
approveToolCall resumption code paths to use this per-request credential
injection.
🧹 Nitpick comments (12)
packages/agent/src/superagent.ts (2)
11-21: Module-level side effects: all infrastructure is instantiated at import time.
PostgresStore,Memory,Agent, andMastraare all created as top-level constants. This means any module that imports frompackages/agent(directly or transitively) will:
- Attempt a PostgreSQL connection immediately
- Fail hard if
DATABASE_URLis missing- Make isolated unit testing difficult without a live database
If this is intentional (e.g., this module is only ever imported in a controlled server context), consider adding a brief comment documenting that assumption. Otherwise, consider lazy initialization via a factory function.
Also applies to: 66-92, 160-196
70-74:requestContext.get("modelId")— consider validating the returned value.Both agents use a dynamic model resolver that trusts whatever string
requestContext.get("modelId")returns. If a consumer setsmodelIdto a non-empty but invalid provider string, the agent will fail at generation time with an opaque error. A lightweight guard (e.g., checking the string matches a known pattern likeprovider/model) would improve debuggability.Also applies to: 164-168
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/types.ts (1)
69-82:MastraChunk.typeis broadly typed asstring.If the set of chunk types is known and finite, a string literal union would provide better type safety and autocomplete for consumers. If the types are open-ended or come from an external library,
stringis acceptable.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/constants.ts (1)
3-8: Hardcoded default model.Consider whether
DEFAULT_MODELshould be configurable (e.g., via user settings or environment) rather than hardcoded to a specific Anthropic model. This is fine for now but may become a maintenance burden as models evolve.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ToolApprovalBar/ToolApprovalBar.tsx (1)
44-49: Unnecessaryas stringcast.
JSON.stringify()already returnsstring; the cast is redundant.- : (JSON.stringify(pendingApproval.args, null, 2) as string)} + : JSON.stringify(pendingApproval.args, null, 2)}apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/ModelPicker/ModelPicker.tsx (1)
69-93:providerToLogocalled redundantly inside the render loop.
providerToLogo(model.provider)on line 72 is called for every model on every render. Since the provider is already the group key, you can compute it once per group.♻️ Suggested optimization
- {Object.entries(groupedModels).map(([provider, providerModels]) => ( - <ModelSelectorGroup key={provider} heading={provider}> - {providerModels.map((model) => { - const logo = providerToLogo(model.provider); + {Object.entries(groupedModels).map(([provider, providerModels]) => { + const logo = providerToLogo(provider); + return ( + <ModelSelectorGroup key={provider} heading={provider}> + {providerModels.map((model) => { return ( <ModelSelectorItem key={model.id}apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/utils/hydrate-messages.ts (1)
82-112: Agent vs. tool-call detection relies solely on name prefix convention.The
tName.startsWith("agent-")check (line 82) determines whether a tool part is rendered as anAgentCallPartor aToolCallPart. If a regular tool name happens to start with"agent-", it will be misclassified. This is likely fine if the naming convention is enforced upstream, but worth noting as a coupling assumption.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx (1)
28-34:renderPartsis recreated every render and passed toAgentCallBlock.The inner
renderPartsfunction capturesisStreamingfrom the closure and is passed as a prop toAgentCallBlock. Since it's not memoized (e.g., viauseCallback),AgentCallBlockwill receive a new function reference on every render, potentially causing unnecessary re-renders of the collapsible tree. For now this is fine given the streaming context, but consider memoizing if performance becomes an issue.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/hooks/useSuperagentStream/useSuperagentStream.ts (1)
367-383:handleTopLevelChunktakes 12 parameters — consider a params object.This standalone function receives 12 positional arguments, making call sites hard to read and error-prone. Wrapping them in a single options object would improve clarity and reduce positional mistakes.
apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (3)
69-75:execSyncblocks the event loop — considerexecFile(async).
safeExecis called up to 3 times ingatherProjectContext(git branch, status, log), each with a 3-second timeout. SinceexecSyncis synchronous, this can block the Node.js event loop for up to 9 seconds in the worst case, stalling all other IPC handlers and subscriptions during that window.Suggested async alternative
-import { execSync } from "node:child_process"; +import { execFile } from "node:child_process"; +import { promisify } from "node:util"; + +const execFileAsync = promisify(execFile); -function safeExec(cmd: string, cwd: string, timeoutMs = 3_000): string { +async function safeExec(cmd: string, cwd: string, timeoutMs = 3_000): Promise<string> { try { - return execSync(cmd, { cwd, timeout: timeoutMs, encoding: "utf-8" }).trim(); + const { stdout } = await execFileAsync("git", cmd.replace(/^git\s+/, "").split(/\s+/), { cwd, timeout: timeoutMs }); + return stdout.trim(); } catch { return ""; } }This would also require making
gatherProjectContextasync.
314-334: Remove or gate debug logging behind a flag.The
[getMessages]debug block iterates messages and logs tool-part JSON to the console on every query call. This is verbose for production use and may inadvertently log sensitive tool arguments or results.
556-557: Misleading operator precedence — use explicit grouping or a ternary.const contextInstructions = projectContext + fileMentionContext || undefined;This evaluates as
(projectContext + fileMentionContext) || undefineddue to+binding tighter than||. It happens to work (empty string concatenation is falsy), but reads as though||applies only tofileMentionContext.Clearer alternative
-const contextInstructions = - projectContext + fileMentionContext || undefined; +const combined = projectContext + fileMentionContext; +const contextInstructions = combined || undefined;
| function parseFileMentions(text: string, cwd: string): FileMention[] { | ||
| // Match @some/path/to/file.ext patterns (must have at least one / or . to avoid false positives) | ||
| const mentionRegex = /@([\w./-]+(?:\/[\w./-]+|\.[\w]+))/g; | ||
| const mentions: FileMention[] = []; | ||
| const seen = new Set<string>(); | ||
|
|
||
| let match: RegExpExecArray | null; | ||
| while ((match = mentionRegex.exec(text)) !== null) { | ||
| const relPath = match[1]; | ||
| if (seen.has(relPath)) continue; | ||
| seen.add(relPath); | ||
|
|
||
| const absPath = join(cwd, relPath); | ||
| const content = safeReadFile(absPath, 50_000); // allow larger files for explicit mentions | ||
| mentions.push({ | ||
| raw: match[0], | ||
| absPath, | ||
| relPath, | ||
| content, | ||
| }); | ||
| } | ||
|
|
||
| return mentions; | ||
| } |
There was a problem hiding this comment.
Path traversal vulnerability in file mention parsing.
join(cwd, relPath) does not confine reads to the project directory. A user message containing @../../etc/passwd will resolve to a path outside cwd and safeReadFile will happily read it (up to 50 KB). The file contents are then injected into the AI prompt context.
Validate that the resolved absolute path is still within cwd after normalization:
Proposed fix
+import { resolve } from "node:path";
+
function parseFileMentions(text: string, cwd: string): FileMention[] {
const mentionRegex = /@([\w./-]+(?:\/[\w./-]+|\.[\w]+))/g;
const mentions: FileMention[] = [];
const seen = new Set<string>();
+ const normalizedCwd = resolve(cwd);
let match: RegExpExecArray | null;
while ((match = mentionRegex.exec(text)) !== null) {
const relPath = match[1];
if (seen.has(relPath)) continue;
seen.add(relPath);
- const absPath = join(cwd, relPath);
+ const absPath = resolve(cwd, relPath);
+ if (!absPath.startsWith(normalizedCwd + "/") && absPath !== normalizedCwd) continue;
const content = safeReadFile(absPath, 50_000);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function parseFileMentions(text: string, cwd: string): FileMention[] { | |
| // Match @some/path/to/file.ext patterns (must have at least one / or . to avoid false positives) | |
| const mentionRegex = /@([\w./-]+(?:\/[\w./-]+|\.[\w]+))/g; | |
| const mentions: FileMention[] = []; | |
| const seen = new Set<string>(); | |
| let match: RegExpExecArray | null; | |
| while ((match = mentionRegex.exec(text)) !== null) { | |
| const relPath = match[1]; | |
| if (seen.has(relPath)) continue; | |
| seen.add(relPath); | |
| const absPath = join(cwd, relPath); | |
| const content = safeReadFile(absPath, 50_000); // allow larger files for explicit mentions | |
| mentions.push({ | |
| raw: match[0], | |
| absPath, | |
| relPath, | |
| content, | |
| }); | |
| } | |
| return mentions; | |
| } | |
| function parseFileMentions(text: string, cwd: string): FileMention[] { | |
| // Match `@some/path/to/file.ext` patterns (must have at least one / or . to avoid false positives) | |
| const mentionRegex = /@([\w./-]+(?:\/[\w./-]+|\.[\w]+))/g; | |
| const mentions: FileMention[] = []; | |
| const seen = new Set<string>(); | |
| const normalizedCwd = resolve(cwd); | |
| let match: RegExpExecArray | null; | |
| while ((match = mentionRegex.exec(text)) !== null) { | |
| const relPath = match[1]; | |
| if (seen.has(relPath)) continue; | |
| seen.add(relPath); | |
| const absPath = resolve(cwd, relPath); | |
| if (!absPath.startsWith(normalizedCwd + "/") && absPath !== normalizedCwd) continue; | |
| const content = safeReadFile(absPath, 50_000); // allow larger files for explicit mentions | |
| mentions.push({ | |
| raw: match[0], | |
| absPath, | |
| relPath, | |
| content, | |
| }); | |
| } | |
| return mentions; | |
| } |
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 198 - 221,
The parseFileMentions function currently uses join(cwd, relPath) and can read
files outside the project (path traversal); after computing the candidate path
(use path.resolve or normalize on join(cwd, relPath)) verify that the resolved
path remains inside cwd (for example by ensuring path.relative(cwd,
resolvedPath) does not start with '..' and is not absolute outside cwd) and skip
adding/reading any mention that escapes cwd; update parseFileMentions to perform
this check before calling safeReadFile so only files under cwd are read.
| sendMessage: publicProcedure | ||
| .input(z.object({ sessionId: z.string(), text: z.string() })) | ||
| .mutation(({ input }) => { | ||
| // Fire-and-forget: agent runs in background, errors surface via streamEvents | ||
| void chatSessionManager | ||
| .startAgent({ | ||
| sessionId: input.sessionId, | ||
| prompt: input.text, | ||
| }) | ||
| .catch((error: unknown) => { | ||
| console.error( | ||
| "[ai-chat/sendMessage] Failed to start agent:", | ||
| error, | ||
| ); | ||
| console.log("sendMessage", input); | ||
| // // Fire-and-forget: agent runs in background, errors surface via streamEvents | ||
| // void chatSessionManager | ||
| // .startAgent({ | ||
| // sessionId: input.sessionId, | ||
| // prompt: input.text, | ||
| // }) | ||
| // .catch((error: unknown) => { | ||
| // console.error( | ||
| // "[ai-chat/sendMessage] Failed to start agent:", | ||
| // error, | ||
| // ); | ||
| // }); | ||
| return { success: true }; | ||
| }), |
There was a problem hiding this comment.
sendMessage is a no-op — remove or restore the implementation.
The entire agent-dispatch logic is commented out, leaving sendMessage as a mutation that only logs and returns { success: true } without doing anything. If this endpoint is still called by the frontend, users will see no response. If it's replaced by superagent, consider removing it to avoid confusion.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 497 - 514,
The sendMessage mutation currently does nothing (just logs and returns {
success: true }); either restore the original agent dispatch by uncommenting and
re-enabling the chatSessionManager.startAgent(...) fire-and-forget call
(including its .catch(...) error logging) and remove the stray console.log, or
delete the sendMessage procedure entirely if the frontend now uses a different
endpoint (e.g., superagent). Locate the sendMessage publicProcedure and either
re-enable the chatSessionManager.startAgent invocation with proper error
handling or remove the procedure to avoid a no-op API surface.
| void (async () => { | ||
| // --- Abort controller --- | ||
| const abortController = new AbortController(); | ||
| sessionAbortControllers.set(input.sessionId, abortController); | ||
|
|
||
| // Store context for approval resumption | ||
| sessionContext.set(input.sessionId, { | ||
| cwd: input.cwd, | ||
| modelId: input.modelId, | ||
| }); |
There was a problem hiding this comment.
Concurrent calls for the same session overwrite the abort controller without cancelling the previous stream.
If superagent is called twice for the same sessionId before the first stream completes, line 539 overwrites the AbortController in the map. The first stream becomes uncancellable but keeps emitting chunks to the same session event, interleaving with the second stream's output.
Consider aborting any existing controller before storing the new one:
Proposed fix
void (async () => {
+ // Cancel any in-flight stream for this session
+ sessionAbortControllers.get(input.sessionId)?.abort();
+
const abortController = new AbortController();
sessionAbortControllers.set(input.sessionId, abortController);🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 536 - 545,
Concurrent requests for the same sessionId overwrite the AbortController in
sessionAbortControllers, leaving the previous stream uncancellable; before
creating and storing a new AbortController in the async block, check
sessionAbortControllers.get(input.sessionId) and if one exists call .abort() on
it and remove it, then create the new AbortController and set it
(sessionAbortControllers.set(input.sessionId, abortController)); ensure any
cleanup also updates sessionAbortControllers (and optionally sessionContext)
when streams finish or error to avoid stale controllers.
| } catch (error) { | ||
| // Don't emit error for intentional aborts | ||
| if (abortController.signal.aborted) { | ||
| console.log("[ai-chat/superagent] Stream aborted by user."); | ||
| superagentEmitter.emit(input.sessionId, { | ||
| type: "done", | ||
| }); | ||
| return; | ||
| } | ||
| console.error("[ai-chat/superagent] Stream failed:", error); | ||
| superagentEmitter.emit(input.sessionId, { | ||
| type: "error", | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } finally { | ||
| sessionAbortControllers.delete(input.sessionId); | ||
| } |
There was a problem hiding this comment.
sessionRunIds and sessionContext leak on stream error.
On a non-abort error, the catch block emits an error event but does not clean up sessionRunIds or sessionContext. The finally block only removes the abort controller. This leaves stale entries that could cause a later approveToolCall to use outdated context or attempt to resume a dead stream.
Proposed fix
} catch (error) {
if (abortController.signal.aborted) {
console.log("[ai-chat/superagent] Stream aborted by user.");
superagentEmitter.emit(input.sessionId, { type: "done" });
return;
}
console.error("[ai-chat/superagent] Stream failed:", error);
superagentEmitter.emit(input.sessionId, {
type: "error",
error: error instanceof Error ? error.message : String(error),
});
} finally {
sessionAbortControllers.delete(input.sessionId);
+ // Clean up on any terminal path (error or abort)
+ // Success + suspension paths handle their own cleanup
+ if (!sessionSuspended.has(input.sessionId)) {
+ sessionRunIds.delete(input.sessionId);
+ sessionContext.delete(input.sessionId);
+ }
}🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 663 - 679,
The catch block for the superagent stream currently emits an error but doesn't
clean up sessionRunIds or sessionContext, causing stale state; update the catch
(error) handler so that if the error is not an intentional abort (i.e.,
abortController.signal.aborted is false) you also delete
sessionRunIds.delete(input.sessionId) and sessionContext.delete(input.sessionId)
(the same keys you remove on successful completion) before emitting the error
event, ensuring sessionRunIds, sessionContext, and sessionAbortControllers are
all cleared when a stream fails.
| useEffect(() => { | ||
| if (!sessionId || !cwd) return; | ||
| if (existingSession === undefined) return; | ||
|
|
||
| hasConnected.current = false; | ||
| setSessionReady(false); | ||
|
|
||
| if (existingSession) { | ||
| restoreSessionRef.current.mutate({ | ||
| sessionId, | ||
| cwd, | ||
| paneId, | ||
| tabId, | ||
| model: selectedModelRef.current.id, | ||
| permissionMode: permissionModeRef.current, | ||
| }); | ||
| } else { | ||
| startSessionRef.current.mutate({ | ||
| sessionId, | ||
| workspaceId, | ||
| cwd, | ||
| paneId, | ||
| tabId, | ||
| model: selectedModelRef.current.id, | ||
| permissionMode: permissionModeRef.current, | ||
| }); | ||
| } | ||
|
|
||
| return () => { | ||
| stopSessionRef.current.mutate({ sessionId }); | ||
| }; | ||
| }, [sessionId, cwd, workspaceId, existingSession, paneId, tabId]); | ||
| if (!historyMessages || historyMessages.length === 0) return; | ||
| setMessages( | ||
| hydrateMessages(historyMessages as Array<Record<string, unknown>>), | ||
| ); | ||
| }, [historyMessages]); |
There was a problem hiding this comment.
History re-fetch during streaming will overwrite local message state.
The useEffect at line 79 calls setMessages(hydrateMessages(...)) whenever historyMessages changes. If the tRPC query refetches (e.g., due to window focus or polling) while the user is mid-stream, it will replace the live-streaming messages with the stale server history, causing the UI to jump or lose in-progress content.
Consider adding a guard (e.g., skip if isStreaming is true) or using { refetchOnWindowFocus: false } on the query.
Proposed fix
useEffect(() => {
if (!historyMessages || historyMessages.length === 0) return;
+ if (isStreaming) return; // Don't overwrite live-streaming state
setMessages(
hydrateMessages(historyMessages as Array<Record<string, unknown>>),
);
- }, [historyMessages]);
+ }, [historyMessages, isStreaming]);📝 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.
| useEffect(() => { | |
| if (!sessionId || !cwd) return; | |
| if (existingSession === undefined) return; | |
| hasConnected.current = false; | |
| setSessionReady(false); | |
| if (existingSession) { | |
| restoreSessionRef.current.mutate({ | |
| sessionId, | |
| cwd, | |
| paneId, | |
| tabId, | |
| model: selectedModelRef.current.id, | |
| permissionMode: permissionModeRef.current, | |
| }); | |
| } else { | |
| startSessionRef.current.mutate({ | |
| sessionId, | |
| workspaceId, | |
| cwd, | |
| paneId, | |
| tabId, | |
| model: selectedModelRef.current.id, | |
| permissionMode: permissionModeRef.current, | |
| }); | |
| } | |
| return () => { | |
| stopSessionRef.current.mutate({ sessionId }); | |
| }; | |
| }, [sessionId, cwd, workspaceId, existingSession, paneId, tabId]); | |
| if (!historyMessages || historyMessages.length === 0) return; | |
| setMessages( | |
| hydrateMessages(historyMessages as Array<Record<string, unknown>>), | |
| ); | |
| }, [historyMessages]); | |
| useEffect(() => { | |
| if (!historyMessages || historyMessages.length === 0) return; | |
| if (isStreaming) return; // Don't overwrite live-streaming state | |
| setMessages( | |
| hydrateMessages(historyMessages as Array<Record<string, unknown>>), | |
| ); | |
| }, [historyMessages, isStreaming]); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`
around lines 79 - 84, The effect that sets local messages from historyMessages
(useEffect that calls setMessages(hydrateMessages(...))) overwrites in-progress
streaming content when history refetches; modify that effect to guard against
overwriting during an active stream (check an isStreaming flag or similar state
before calling setMessages) or alternatively disable unwanted refetching at the
source (set the tRPC query option refetchOnWindowFocus: false / disable polling)
so hydrateMessages is only applied when not streaming.
| // Debug: log the raw shape from the server to verify toAISdkV5Messages output | ||
| console.log( | ||
| "[hydrate] raw historyMessages sample:", | ||
| JSON.stringify( | ||
| historyMessages.slice(0, 2).map((m) => ({ | ||
| id: m.id, | ||
| role: m.role, | ||
| hasTopLevelParts: Array.isArray(m.parts), | ||
| hasContentParts: | ||
| !!m.content && | ||
| Array.isArray((m.content as Record<string, unknown>)?.parts), | ||
| partTypes: ( | ||
| (m.parts as Array<Record<string, unknown>>) ?? | ||
| ((m.content as Record<string, unknown>)?.parts as Array< | ||
| Record<string, unknown> | ||
| >) ?? | ||
| [] | ||
| ).map((p) => ({ type: p.type, toolName: p.toolName })), | ||
| })), | ||
| null, | ||
| 2, | ||
| ), | ||
| ); |
There was a problem hiding this comment.
Remove debug console.log — leaks message content in production.
This logs the first two raw messages (including user text, tool arguments, and potentially sensitive data) every time history is hydrated. This should be removed or gated behind a debug flag before merging.
Proposed fix
- // Debug: log the raw shape from the server to verify toAISdkV5Messages output
- console.log(
- "[hydrate] raw historyMessages sample:",
- JSON.stringify(
- historyMessages.slice(0, 2).map((m) => ({
- id: m.id,
- role: m.role,
- hasTopLevelParts: Array.isArray(m.parts),
- hasContentParts:
- !!m.content &&
- Array.isArray((m.content as Record<string, unknown>)?.parts),
- partTypes: (
- (m.parts as Array<Record<string, unknown>>) ??
- ((m.content as Record<string, unknown>)?.parts as Array<
- Record<string, unknown>
- >) ??
- []
- ).map((p) => ({ type: p.type, toolName: p.toolName })),
- })),
- null,
- 2,
- ),
- );📝 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.
| // Debug: log the raw shape from the server to verify toAISdkV5Messages output | |
| console.log( | |
| "[hydrate] raw historyMessages sample:", | |
| JSON.stringify( | |
| historyMessages.slice(0, 2).map((m) => ({ | |
| id: m.id, | |
| role: m.role, | |
| hasTopLevelParts: Array.isArray(m.parts), | |
| hasContentParts: | |
| !!m.content && | |
| Array.isArray((m.content as Record<string, unknown>)?.parts), | |
| partTypes: ( | |
| (m.parts as Array<Record<string, unknown>>) ?? | |
| ((m.content as Record<string, unknown>)?.parts as Array< | |
| Record<string, unknown> | |
| >) ?? | |
| [] | |
| ).map((p) => ({ type: p.type, toolName: p.toolName })), | |
| })), | |
| null, | |
| 2, | |
| ), | |
| ); |
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/utils/hydrate-messages.ts`
around lines 10 - 32, The debug console.log in hydrate-messages.ts is leaking
raw message content; remove the console.log call that prints historyMessages
(the JSON.stringify block referencing historyMessages.slice(...)) or wrap it
behind a proper debug flag/logger check (e.g., only log when DEBUG or a dev-only
logger is enabled). Locate the console.log in the hydrate messages utility and
either delete it or replace it with a conditional debug-level log using your
app's logger (so production builds never print raw message content).
| export function toToolDisplayState(part: ToolCallPart): ToolDisplayState { | ||
| if (part.status === "streaming") return "input-streaming"; | ||
| if (part.status === "calling") return "input-complete"; | ||
| if (part.isError) return "output-error"; | ||
| if (part.result != null) return "output-available"; | ||
| return "input-available"; | ||
| } |
There was a problem hiding this comment.
Edge case: status === "done" with no result and no error falls through to "input-available".
If a tool call completes (status: "done") but has neither result nor isError set, both toToolDisplayState and toWsToolState will return "input-available", which semantically suggests the tool hasn't started yet. This is likely a rare edge case, but worth considering whether a completed-but-empty-result state should map to something else (e.g., "output-available").
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/utils/tool-helpers.ts`
around lines 4 - 10, Handle the edge case where a ToolCallPart has status ===
"done" but no result and no isError: update toToolDisplayState to check for
part.status === "done" (before the final fallthrough) and return
"output-available" for a completed-but-empty-result call; mirror the same logic
in toWsToolState so both mappings treat a finished-but-empty tool call as an
output-available state rather than "input-available".
| initSessionStore, | ||
| setClaudeSessionId, | ||
| } from "./session-store"; | ||
| export { memory, superagent } from "./superagent"; |
There was a problem hiding this comment.
Re-exporting ./superagent triggers module-level side effects for all consumers of @superset/agent.
Because superagent.ts instantiates PostgresStore at the top level, every consumer that imports anything from @superset/agent — even unrelated exports like createPermissionRequest or getActiveSessionCount — will now require DATABASE_URL to be set and will attempt a PostgreSQL connection at import time.
If not all consumers need the superagent, consider isolating it behind a separate export path (e.g., @superset/agent/superagent) or using lazy initialization in superagent.ts.
🤖 Prompt for AI Agents
In `@packages/agent/src/index.ts` at line 15, index.ts re-exporting "./superagent"
causes PostgresStore to be instantiated at module import, forcing
DATABASE_URL/DB connection for all consumers; remove the direct re-export from
packages/agent/src/index.ts and either (A) expose superagent via a separate
entrypoint (e.g., create a dedicated export path "@superset/agent/superagent"
that re-exports the module) or (B) change superagent.ts to lazy-initialize
PostgresStore (provide a getSuperagent/getStore factory function that constructs
PostgresStore on first use instead of at top-level). Update any callers to
import the new entrypoint or call the factory (referencing ./superagent,
PostgresStore, and the current top-level exports like
createPermissionRequest/getActiveSessionCount to ensure they no longer trigger
DB initialization).
| export const storage = new PostgresStore({ | ||
| connectionString: process.env.DATABASE_URL!, | ||
| id: "superagent-db", | ||
| }); |
There was a problem hiding this comment.
process.env.DATABASE_URL! — non-null assertion flagged by pipeline lint.
The linter warns against the non-null assertion. If DATABASE_URL is unset, undefined is silently passed to PostgresStore, likely causing a confusing runtime error (connection failure rather than a clear "missing config" message). Since this runs at module-load time, the failure surface is wide.
Proposed fix
+const connectionString = process.env.DATABASE_URL;
+if (!connectionString) {
+ throw new Error("DATABASE_URL environment variable is required for superagent storage");
+}
+
export const storage = new PostgresStore({
- connectionString: process.env.DATABASE_URL!,
+ connectionString,
id: "superagent-db",
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export const storage = new PostgresStore({ | |
| connectionString: process.env.DATABASE_URL!, | |
| id: "superagent-db", | |
| }); | |
| const connectionString = process.env.DATABASE_URL; | |
| if (!connectionString) { | |
| throw new Error("DATABASE_URL environment variable is required for superagent storage"); | |
| } | |
| export const storage = new PostgresStore({ | |
| connectionString, | |
| id: "superagent-db", | |
| }); |
🧰 Tools
🪛 GitHub Actions: CI
[warning] 12-12: lint/style/noNonNullAssertion: Forbidden non-null assertion. connectionString: process.env.DATABASE_URL!
🤖 Prompt for AI Agents
In `@packages/agent/src/superagent.ts` around lines 11 - 14, The module currently
uses a non-null assertion on process.env.DATABASE_URL when constructing storage
with new PostgresStore, which the linter flags and which can produce unclear
runtime failures at module load; update the module to explicitly validate that
process.env.DATABASE_URL is set before creating storage (e.g., read
process.env.DATABASE_URL into a local variable, if falsy throw a clear Error
like "DATABASE_URL is required" or return a guarded factory), and then pass that
validated string into new PostgresStore so storage is only constructed with a
guaranteed connection string.
| - In the `superagent` mutation (line 522), before calling `superagent.stream()`: | ||
| 1. Parse the provider from `modelId` (e.g., `"anthropic/..."` → `"anthropic"`) | ||
| 2. Call the credential store to get a valid access token for that provider | ||
| 3. If found, set `process.env.ANTHROPIC_API_KEY` (or `OPENAI_API_KEY`) to the token | ||
| 4. After the stream completes, restore the original env var value | ||
| - Same for `approveToolCall` resumption path |
There was a problem hiding this comment.
Race condition risk in process.env token injection strategy.
The plan injects OAuth tokens into process.env before each superagent.stream() call and restores afterwards. Since Node.js is single-threaded but async, if two concurrent sessions trigger overlapping agent calls for different providers/tokens, one session's env var write can be read by another session's Mastra call between the set and restore. Consider passing credentials directly to the Mastra agent/model constructor (if supported) rather than mutating a shared global.
🤖 Prompt for AI Agents
In `@plan.md` around lines 46 - 51, The current approach mutates process.env
(e.g., setting process.env.ANTHROPIC_API_KEY or process.env.OPENAI_API_KEY)
around calls in the superagent mutation (before superagent.stream() at the code
referenced around line 522) and the approveToolCall resumption path, which
creates a race; instead, refactor so you do NOT write to process.env: retrieve
the provider token from the credential store, then pass that token directly into
the Mastra agent/model constructor or into superagent.stream()/request options
(or whatever per-request auth API the Mastra/agent client exposes) for the
specific call (use the parsed provider from modelId such as "anthropic" to pick
token), and remove the set/restore of the env var; update both the superagent
mutation and the approveToolCall resumption code paths to use this per-request
credential injection.
The heredoc was writing literal \t strings instead of tab characters, causing Caddy to fail with "unrecognized directive: \treverse_proxy". Switch to <<- heredoc with actual tab indentation.
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Fix all issues with AI agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts`:
- Around line 46-60: The per-session collections sessionAbortControllers,
sessionRunIds, sessionContext, and sessionSuspended are never cleared, causing a
leak; update the session teardown paths (deleteSession and stopSession which
call chatSessionManager) to remove any entries for the given sessionId by
calling sessionAbortControllers.delete(sessionId),
sessionRunIds.delete(sessionId), sessionContext.delete(sessionId), and
sessionSuspended.delete(sessionId) (also remove any listeners on
superagentEmitter for that session if applicable) so state is fully cleaned when
a session is stopped or deleted.
- Around line 77-83: safeExec currently uses execSync which blocks the Electron
main process; replace it with an async implementation (e.g., promisified
child_process.execFile or using child_process.spawn wrapped in a Promise) and
make safeExec return a Promise<string>; update any callers such as
gatherProjectContext (make it async) and the superagent handler that invokes it
to await the new safeExec calls (for git branch/status/log) so the commands run
asynchronously and do not block the event loop.
- Around line 763-827: The resumed stream started in the approveToolCall block
doesn't register an AbortController so subsequent abortSuperagent calls can't
cancel it; create a new AbortController before calling
superagent.approveToolCall / declineToolCall, add its signal to approvalOpts
(e.g., approvalOpts.signal = controller.signal), store the controller in
sessionAbortControllers keyed by input.sessionId, and ensure you clean it up
(delete from sessionAbortControllers) when the resumed stream completes or
errors (similar to the original stream's finally) so abortSuperagent can cancel
this resumed stream.
In `@packages/agent/src/superagent.ts`:
- Around line 125-136: The Workspace creation is using unvalidated cwd from user
input when constructing LocalFilesystem and LocalSandbox (seen in Workspace,
LocalFilesystem, LocalSandbox usage), which can allow path traversal or access
to arbitrary system paths; before creating the Workspace (or before calling
executeAgent in the ai-chat router), call the existing
assertRegisteredWorktree(cwd) validation from path-validation.ts (or otherwise
map/normalize cwd to a canonical registered worktree root) and reject or
normalize any cwd that fails validation; apply the same guard where similar
Workspace construction occurs (also around the block at lines ~214-219) so only
registered workspace roots are used as basePath/workingDirectory.
🧹 Nitpick comments (8)
apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MessagePartsRenderer/MessagePartsRenderer.tsx (1)
30-36:renderPartsinconsistently mixes explicit parameters with closure captures.
partsandisLastAssistantare explicit parameters (allowingAgentCallBlockto supply its own values), butisStreamingandonAnswerare captured from the parent closure. If a nested agent call needs a differentisStreamingoronAnswer, there's no way to pass it. Consider either making all four values explicit parameters, or documenting that the closure capture is intentional.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/components/MastraToolCallBlock/MastraToolCallBlock.tsx (2)
126-141: Unsafe type assertion onresult.answers.
result.answers as Record<string, string> | undefinedbypasses runtime validation. If the server ever returns non-string values inanswers, this will silently propagate bad data. Consider a lightweight runtime check or at minimum narrowing withtypeof.Proposed safer narrowing
- const answers = result.answers as - | Record<string, string> - | undefined; + const answers = + result.answers != null && + typeof result.answers === "object" && + !Array.isArray(result.answers) + ? (result.answers as Record<string, string>) + : undefined;As per coding guidelines, "Avoid
anytype annotation unless necessary - prioritize type safety".
158-178: Fallback uses rawpart.args/part.resultwhile other branches usegetArgs/getResulthelpers.This is likely intentional for generic display, but worth noting: if
getArgs/getResultdo meaningful normalization (e.g., JSON parsing), the fallbackToolInput/ToolOutputmay receive unparsed data. If the genericToolcomponents can handle both parsed and raw forms, this is fine.packages/agent/src/superagent.ts (2)
62-72: Eager module-level side effects:PostgresStoreandMemoryinstantiate on import.Both
storageandmemoryare created at module load time, meaning any file that imports anything from this module will trigger a Postgres connection attempt — even if the consumer only needs, say,setAnthropicAuthToken. If this module is ever imported in a context whereDATABASE_URLis unavailable (tests, CLI tooling, client bundles), it will fail immediately.Consider lazy initialization (factory function or getter) so the connection is only established when actually needed.
122-124: Redundant type assertion:as string | undefined.
requestContext.get()already returnsstring | undefinedper the type declared on line 39. Theascast is unnecessary and can mask future type changes.- const cwd = requestContext.get("cwd") as string | undefined; + const cwd = requestContext.get("cwd");Also applies to: 211-213
apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (3)
333-353: Remove debug logging fromgetMessages.This block logs tool-part keys and up to 500 chars of serialized message content to the console on every
getMessagescall. It appears to be development scaffolding — it adds per-request overhead and may leak conversation content into logs.Proposed fix
const uiMessages = toAISdkV5Messages(result.messages); - // Debug: log tool parts to verify shape - for (const msg of uiMessages.slice(0, 3)) { - const parts = (msg as Record<string, unknown>).parts as - | Array<Record<string, unknown>> - | undefined; - if (parts) { - for (const p of parts) { - if (String(p.type ?? "").startsWith("tool-")) { - console.log( - "[getMessages] V5 tool part keys:", - Object.keys(p), - "type:", - p.type, - ); - console.log( - "[getMessages] V5 tool part:", - JSON.stringify(p, null, 2).slice(0, 500), - ); - } - } - } - } return uiMessages;
574-576: Operator-precedence gotcha — consider explicit clarity.const contextInstructions = projectContext + fileMentionContext || undefined;This reads as
(projectContext + fileMentionContext) || undefineddue to+binding tighter than||, which happens to produce the desired result. However, it's easy to misread on a quick scan. A small clarity improvement:Proposed fix
- const contextInstructions = - projectContext + fileMentionContext || undefined; + const combinedContext = projectContext + fileMentionContext; + const contextInstructions = combinedContext || undefined;
629-672: Reduce verbose per-chunk logging before shipping.The streaming loop logs on every chunk that matches a broad set of types (tool-*, text-delta, finish, start, step-start, step-finish) and on the first 5 chunks unconditionally. For active sessions, this produces a very high volume of console output. Consider limiting to lifecycle events (start/finish/error/suspension) only, or gating behind a
DEBUGflag.
| /** Per-session event bus for streaming superagent chunks to the renderer */ | ||
| const superagentEmitter = new EventEmitter(); | ||
| superagentEmitter.setMaxListeners(50); | ||
|
|
||
| /** Per-session AbortController for cancelling running streams */ | ||
| const sessionAbortControllers = new Map<string, AbortController>(); | ||
|
|
||
| /** Per-session runId for tool approval (maps sessionId → runId) */ | ||
| const sessionRunIds = new Map<string, string>(); | ||
|
|
||
| /** Per-session context needed for approval resumption (maps sessionId → { cwd, modelId }) */ | ||
| const sessionContext = new Map<string, { cwd: string; modelId: string }>(); | ||
|
|
||
| /** Track whether a session stream is suspended (waiting for tool approval) */ | ||
| const sessionSuspended = new Set<string>(); |
There was a problem hiding this comment.
Per-session maps are never cleaned up by deleteSession / stopSession.
The module-level maps (sessionAbortControllers, sessionRunIds, sessionContext) and sessionSuspended set grow over the application lifetime. The deleteSession (line 429) and stopSession (line 420) procedures delegate to chatSessionManager but never clear entries from these maps. In a long-running Electron process, this is a slow memory leak and can also cause stale state if a session ID is reused.
Consider adding cleanup when sessions are explicitly stopped or deleted:
Proposed fix (in deleteSession / stopSession)
deleteSession: publicProcedure
.input(z.object({ sessionId: z.string() }))
.mutation(async ({ input }) => {
+ sessionAbortControllers.get(input.sessionId)?.abort();
+ sessionAbortControllers.delete(input.sessionId);
+ sessionRunIds.delete(input.sessionId);
+ sessionContext.delete(input.sessionId);
+ sessionSuspended.delete(input.sessionId);
await chatSessionManager.deleteSession({
sessionId: input.sessionId,
});
return { success: true };
}),Apply similarly to stopSession.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 46 - 60, The
per-session collections sessionAbortControllers, sessionRunIds, sessionContext,
and sessionSuspended are never cleared, causing a leak; update the session
teardown paths (deleteSession and stopSession which call chatSessionManager) to
remove any entries for the given sessionId by calling
sessionAbortControllers.delete(sessionId), sessionRunIds.delete(sessionId),
sessionContext.delete(sessionId), and sessionSuspended.delete(sessionId) (also
remove any listeners on superagentEmitter for that session if applicable) so
state is fully cleaned when a session is stopped or deleted.
| function safeExec(cmd: string, cwd: string, timeoutMs = 3_000): string { | ||
| try { | ||
| return execSync(cmd, { cwd, timeout: timeoutMs, encoding: "utf-8" }).trim(); | ||
| } catch { | ||
| return ""; | ||
| } | ||
| } |
There was a problem hiding this comment.
execSync blocks the Electron main-process event loop.
safeExec uses execSync, which is called up to 3 times from gatherProjectContext (git branch, git status, git log) on every superagent invocation. Each call can block for up to 3 seconds (the configured timeout), stalling all IPC, tRPC handlers, and window responsiveness for up to ~9 seconds in the worst case.
Switch to an async alternative (child_process.execFile with util.promisify, or child_process.spawn) and await it inside the already-async superagent handler.
Proposed fix sketch
+import { execFile } from "node:child_process";
+import { promisify } from "node:util";
+const execFileAsync = promisify(execFile);
+
-function safeExec(cmd: string, cwd: string, timeoutMs = 3_000): string {
+async function safeExec(cmd: string, cwd: string, timeoutMs = 3_000): Promise<string> {
try {
- return execSync(cmd, { cwd, timeout: timeoutMs, encoding: "utf-8" }).trim();
+ const { stdout } = await execFileAsync("sh", ["-c", cmd], {
+ cwd,
+ timeout: timeoutMs,
+ encoding: "utf-8",
+ });
+ return stdout.trim();
} catch {
return "";
}
}gatherProjectContext would then become async as well.
🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 77 - 83,
safeExec currently uses execSync which blocks the Electron main process; replace
it with an async implementation (e.g., promisified child_process.execFile or
using child_process.spawn wrapped in a Promise) and make safeExec return a
Promise<string>; update any callers such as gatherProjectContext (make it async)
and the superagent handler that invokes it to await the new safeExec calls (for
git branch/status/log) so the commands run asynchronously and do not block the
event loop.
| void (async () => { | ||
| try { | ||
| // Restore the requestContext so dynamic workspace tools are available | ||
| const ctx = sessionContext.get(input.sessionId); | ||
| const reqCtx = ctx | ||
| ? new RequestContext([ | ||
| ["modelId", ctx.modelId], | ||
| ["cwd", ctx.cwd], | ||
| ]) | ||
| : undefined; | ||
|
|
||
| console.log( | ||
| `[ai-chat/approveToolCall] Calling ${input.approved ? "approveToolCall" : "declineToolCall"} with runId: ${input.runId}, cwd: ${ctx?.cwd}`, | ||
| ); | ||
| const approvalOpts = { | ||
| runId: input.runId, | ||
| ...(reqCtx ? { requestContext: reqCtx } : {}), | ||
| }; | ||
| const stream = input.approved | ||
| ? await superagent.approveToolCall(approvalOpts) | ||
| : await superagent.declineToolCall(approvalOpts); | ||
|
|
||
| let chunkCount = 0; | ||
| console.log( | ||
| `[ai-chat/approveToolCall] Got stream, starting to iterate. Listeners: ${superagentEmitter.listenerCount(input.sessionId)}`, | ||
| ); | ||
|
|
||
| for await (const chunk of stream.fullStream) { | ||
| chunkCount++; | ||
| const c = chunk as { type?: string }; | ||
|
|
||
| // Check if the resumed stream itself hits another approval | ||
| if (c.type === "tool-call-approval") { | ||
| sessionSuspended.add(input.sessionId); | ||
| console.log( | ||
| `[ai-chat/approveToolCall] Another tool approval required`, | ||
| ); | ||
| } | ||
|
|
||
| superagentEmitter.emit(input.sessionId, { | ||
| type: "chunk", | ||
| chunk, | ||
| }); | ||
| } | ||
|
|
||
| console.log( | ||
| `[ai-chat/approveToolCall] Resumed stream complete. Chunks: ${chunkCount}`, | ||
| ); | ||
|
|
||
| // Only emit done if not suspended again | ||
| if (!sessionSuspended.has(input.sessionId)) { | ||
| superagentEmitter.emit(input.sessionId, { | ||
| type: "done", | ||
| }); | ||
| sessionRunIds.delete(input.sessionId); | ||
| sessionContext.delete(input.sessionId); | ||
| } | ||
| } catch (error) { | ||
| console.error("[ai-chat/approveToolCall] Stream failed:", error); | ||
| superagentEmitter.emit(input.sessionId, { | ||
| type: "error", | ||
| error: error instanceof Error ? error.message : String(error), | ||
| }); | ||
| } | ||
| })(); |
There was a problem hiding this comment.
Resumed stream is not cancellable — no AbortController registered.
When approveToolCall fires the resumed stream, it doesn't create or register a new AbortController in sessionAbortControllers. The original controller was already deleted in the first stream's finally block. If the user calls abortSuperagent while the resumed stream is iterating, nothing happens — the stream keeps running.
Proposed fix
void (async () => {
+ const abortController = new AbortController();
+ sessionAbortControllers.set(input.sessionId, abortController);
+
try {
const ctx = sessionContext.get(input.sessionId);
const reqCtx = ctx
? new RequestContext([
["modelId", ctx.modelId],
["cwd", ctx.cwd],
])
: undefined;
// ... existing code ...
+ } catch (error) {
+ console.error("[ai-chat/approveToolCall] Stream failed:", error);
+ superagentEmitter.emit(input.sessionId, {
+ type: "error",
+ error: error instanceof Error ? error.message : String(error),
+ });
+ } finally {
+ sessionAbortControllers.delete(input.sessionId);
+ if (!sessionSuspended.has(input.sessionId)) {
+ sessionRunIds.delete(input.sessionId);
+ sessionContext.delete(input.sessionId);
+ }
}🤖 Prompt for AI Agents
In `@apps/desktop/src/lib/trpc/routers/ai-chat/index.ts` around lines 763 - 827,
The resumed stream started in the approveToolCall block doesn't register an
AbortController so subsequent abortSuperagent calls can't cancel it; create a
new AbortController before calling superagent.approveToolCall / declineToolCall,
add its signal to approvalOpts (e.g., approvalOpts.signal = controller.signal),
store the controller in sessionAbortControllers keyed by input.sessionId, and
ensure you clean it up (delete from sessionAbortControllers) when the resumed
stream completes or errors (similar to the original stream's finally) so
abortSuperagent can cancel this resumed stream.
| return new Workspace({ | ||
| id: `planner-workspace-${cwd}`, | ||
| name: `${cwd} (read-only)`, | ||
| filesystem: new LocalFilesystem({ basePath: cwd }), | ||
| sandbox: new LocalSandbox({ workingDirectory: cwd }), | ||
| tools: { | ||
| mastra_workspace_write_file: { enabled: false }, | ||
| mastra_workspace_edit_file: { enabled: false }, | ||
| mastra_workspace_delete: { enabled: false }, | ||
| mastra_workspace_mkdir: { enabled: false }, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# First, locate and examine the superagent.ts file
find . -name "superagent.ts" -type fRepository: superset-sh/superset
Length of output: 98
🏁 Script executed:
#!/bin/bash
# Get the content around lines 125-136 and 214-219
cat -n packages/agent/src/superagent.ts | sed -n '120,140p'
cat -n packages/agent/src/superagent.ts | sed -n '210,225p'Repository: superset-sh/superset
Length of output: 1412
🏁 Script executed:
#!/bin/bash
# Search for how requestContext is created and how cwd is set
rg -n "requestContext" packages/agent/src/superagent.ts -A 2 -B 2Repository: superset-sh/superset
Length of output: 791
🏁 Script executed:
#!/bin/bash
# Search for validation of cwd anywhere in the codebase
rg -n "cwd" packages/agent/src/ --type=ts -A 3 -B 3 | head -100Repository: superset-sh/superset
Length of output: 3153
🏁 Script executed:
#!/bin/bash
# Look for the LocalFilesystem and LocalSandbox implementations
fd -e ts -e tsx | xargs rg -l "class LocalFilesystem|export.*LocalFilesystem"
fd -e ts -e tsx | xargs rg -l "class LocalSandbox|export.*LocalSandbox"Repository: superset-sh/superset
Length of output: 48
🏁 Script executed:
#!/bin/bash
# Search for LocalFilesystem and LocalSandbox more broadly
rg -n "LocalFilesystem|LocalSandbox" packages/ --type=tsRepository: superset-sh/superset
Length of output: 538
🏁 Script executed:
#!/bin/bash
# Check the imports in superagent.ts to find where LocalFilesystem/LocalSandbox come from
head -50 packages/agent/src/superagent.tsRepository: superset-sh/superset
Length of output: 1824
🏁 Script executed:
#!/bin/bash
# Search across the entire codebase for these classes
rg "export.*class.*LocalFilesystem|export.*LocalFilesystem" --type=ts
rg "export.*class.*LocalSandbox|export.*LocalSandbox" --type=tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Look in the entire project for these imports/classes
fd -e ts -e tsx | xargs rg -l "LocalFilesystem" | head -10
fd -e ts -e tsx | xargs rg -l "LocalSandbox" | head -10Repository: superset-sh/superset
Length of output: 129
🏁 Script executed:
#!/bin/bash
# Search for where requestContext is created/populated with cwd
rg -n "requestContext.*cwd|cwd.*requestContext" packages/ --type=ts -A 3 -B 3Repository: superset-sh/superset
Length of output: 1061
🏁 Script executed:
#!/bin/bash
# Look for where the agents are called/invoked to see how requestContext is set
rg -n "executeAgent|runAgent|invoke" packages/ --type=ts -A 5 -B 2 | head -80Repository: superset-sh/superset
Length of output: 3459
🏁 Script executed:
#!/bin/bash
# Find API endpoints or entry points that might accept cwd parameter
rg -n "cwd" packages/ --type=ts -B 5 | grep -E "(app\.|router\.|post|get|export|function)" | head -40Repository: superset-sh/superset
Length of output: 349
🏁 Script executed:
#!/bin/bash
# Check agent-executor.ts more thoroughly to understand the flow
cat -n packages/agent/src/agent-executor.ts | head -100Repository: superset-sh/superset
Length of output: 3148
🏁 Script executed:
#!/bin/bash
# Check the ExecuteAgentParams type definition
cat -n packages/agent/src/types.ts | head -50Repository: superset-sh/superset
Length of output: 1569
🏁 Script executed:
#!/bin/bash
# Search for API routes or endpoints that call executeAgent
rg -n "executeAgent" packages/ --type=ts -B 5 -A 5 | head -100Repository: superset-sh/superset
Length of output: 1329
🏁 Script executed:
#!/bin/bash
# Look for any path validation or sanitization functions in the codebase
rg -n "normalize|resolve|sanitize|validate.*path|allowlist|basePath" packages/ --type=ts | head -40Repository: superset-sh/superset
Length of output: 3558
🏁 Script executed:
#!/bin/bash
# Check if there are any tests that show how cwd is used
fd -e test.ts -e spec.ts | xargs rg -l "cwd|executeAgent" | head -5Repository: superset-sh/superset
Length of output: 382
🏁 Script executed:
#!/bin/bash
# Search for any API routes that accept or handle the cwd parameter
rg -n "cwd" packages/ --type=ts -B 10 | grep -E "(app\.|post|get|route|endpoint|api)" | head -40Repository: superset-sh/superset
Length of output: 263
🏁 Script executed:
#!/bin/bash
# Look for any path normalization or resolution in the codebase
rg -n "path\.resolve|path\.normalize|realpath" packages/agent/ --type=tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check the actual test files to see how cwd is used
cat -n packages/shared/src/terminal-link-parsing/link-parsing.test.ts | head -50Repository: superset-sh/superset
Length of output: 1980
🏁 Script executed:
#!/bin/bash
# Search for the Mastra library documentation or usage of LocalFilesystem
rg -n "@mastra" packages/agent/src/ --type=ts -A 2Repository: superset-sh/superset
Length of output: 1082
🏁 Script executed:
#!/bin/bash
# Look for any documentation or comments about path validation
rg -n "directory|path|traversal|security|validate" packages/agent/src/superagent.ts -i -B 2 -A 2Repository: superset-sh/superset
Length of output: 2349
🏁 Script executed:
#!/bin/bash
# Search for where executeAgent is called to understand the entry point
rg -n "executeAgent\(" packages/ apps/ --type=ts -B 10 | head -80Repository: superset-sh/superset
Length of output: 1964
🏁 Script executed:
#!/bin/bash
# Look for API routes or handlers that might receive cwd from users
find packages apps -name "*.ts" -o -name "*.tsx" | xargs rg -l "cwd" | xargs rg -B 15 "cwd" | grep -E "(route|handler|post|get|api)" | head -30Repository: superset-sh/superset
Length of output: 3088
🏁 Script executed:
#!/bin/bash
# Check if there's any validation in the Anthropic SDK or Mastra library being used
rg -n "LocalFilesystem|LocalSandbox" packages/ --type=ts -A 10 | head -60Repository: superset-sh/superset
Length of output: 2389
🏁 Script executed:
#!/bin/bash
# Look at the complete superagent.ts to understand the context better
wc -l packages/agent/src/superagent.tsRepository: superset-sh/superset
Length of output: 100
🏁 Script executed:
#!/bin/bash
# Check what the mastra libraries do with basePath/workingDirectory
rg -n "basePath|workingDirectory" packages/ --type=tsRepository: superset-sh/superset
Length of output: 435
🏁 Script executed:
#!/bin/bash
# Look at the agent-execution.ts file to see how cwd is passed to executeAgent
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/agent-execution.ts | sed -n '60,100p'Repository: superset-sh/superset
Length of output: 1369
🏁 Script executed:
#!/bin/bash
# Search for where the cwd value is obtained in the desktop app
rg -n "cwd" apps/desktop/src/lib/trpc/routers/ai-chat/ --type=ts -B 5 -A 5 | head -100Repository: superset-sh/superset
Length of output: 8133
🏁 Script executed:
#!/bin/bash
# Look at the ExecuteAgentInput type to understand where cwd originates
rg -n "ExecuteAgentInput|interface.*Input" apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/ --type=ts -A 10 | head -60Repository: superset-sh/superset
Length of output: 6337
🏁 Script executed:
#!/bin/bash
# Check if there's any validation of cwd in the desktop app or elsewhere
rg -n "cwd.*validate|validate.*cwd|path.*allowed|allowlist" packages/ apps/ --type=tsRepository: superset-sh/superset
Length of output: 2264
🏁 Script executed:
#!/bin/bash
# Look at the path-validation.ts file to understand path validation mechanisms
cat -n apps/desktop/src/lib/trpc/routers/changes/security/path-validation.ts | head -180Repository: superset-sh/superset
Length of output: 6042
🏁 Script executed:
#!/bin/bash
# Check where cwd originates in the session lifecycle
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-lifecycle.ts | sed -n '1,80p'Repository: superset-sh/superset
Length of output: 2455
🏁 Script executed:
#!/bin/bash
# Search for where StartSessionInput is created in the router
rg -n "StartSessionInput|startSession" apps/desktop/src/lib/trpc/routers/ai-chat/ --type=ts -B 5 -A 8 | head -80Repository: superset-sh/superset
Length of output: 7704
🏁 Script executed:
#!/antml/bash
# Check if there's validation of cwd in the AI chat router
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/index.ts | sed -n '200,350p'Repository: superset-sh/superset
Length of output: 5451
🏁 Script executed:
#!/antml/bash
# Check if there's any validation of cwd between the API input and executeAgent call
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-lifecycle.ts | sed -n '173,250p'Repository: superset-sh/superset
Length of output: 2437
🏁 Script executed:
#!/bin/bash
# Search for any use of assertRegisteredWorktree in the ai-chat router
rg -n "assertRegisteredWorktree|validateRelativePath|RegisteredWorktree" apps/desktop/src/lib/trpc/routers/ai-chat/ --type=tsRepository: superset-sh/superset
Length of output: 46
🏁 Script executed:
#!/bin/bash
# Check if there's validation in the startSession implementation
cat -n apps/desktop/src/lib/trpc/routers/ai-chat/utils/session-manager/session-lifecycle.ts | sed -n '173,220p'Repository: superset-sh/superset
Length of output: 1512
🏁 Script executed:
#!/antml/bash
# Check if there are any comments or TODO about cwd validation in ai-chat
rg -n "TODO|FIXME|security|validate.*cwd|sanitize.*cwd" apps/desktop/src/lib/trpc/routers/ai-chat/ --type=ts -iRepository: superset-sh/superset
Length of output: 456
Validate cwd against registered worktrees before passing to LocalFilesystem / LocalSandbox.
cwd is accepted directly from user input (z.string() at line 368 of the tRPC router) without validation, then passed through the session lifecycle to superagent.ts. An attacker could provide paths like /etc, ~/.ssh, or traversal sequences to access sensitive files outside intended workspace boundaries.
The codebase already has validation infrastructure (assertRegisteredWorktree() in path-validation.ts) used for file operations in other routers. Apply similar validation in the ai-chat router before storing or passing cwd to executeAgent(), or add safeguards in superagent.ts to restrict basePath and workingDirectory to known workspace roots.
Also applies to: 214-219
🤖 Prompt for AI Agents
In `@packages/agent/src/superagent.ts` around lines 125 - 136, The Workspace
creation is using unvalidated cwd from user input when constructing
LocalFilesystem and LocalSandbox (seen in Workspace, LocalFilesystem,
LocalSandbox usage), which can allow path traversal or access to arbitrary
system paths; before creating the Workspace (or before calling executeAgent in
the ai-chat router), call the existing assertRegisteredWorktree(cwd) validation
from path-validation.ts (or otherwise map/normalize cwd to a canonical
registered worktree root) and reject or normalize any cwd that fails validation;
apply the same guard where similar Workspace construction occurs (also around
the block at lines ~214-219) so only registered workspace roots are used as
basePath/workingDirectory.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/agent/src/superagent.ts`:
- Around line 50-59: The hard-coded deceptive client identity is set in the
createAnthropic call: update the headers passed to createAnthropic (where
anthropicAuthToken and model are used) to stop impersonating the official Claude
CLI; keep the required "anthropic-beta" header but replace "user-agent":
"claude-cli/2.1.2 (external, cli)" and "x-app": "cli" with truthful identifiers
(for example "user-agent": "superagent/1.0" and "x-app": "superagent" or remove
x-app entirely), so the client accurately represents itself while preserving
OAuth beta flags.
🧹 Nitpick comments (3)
packages/agent/src/superagent.ts (3)
21-25: Module-level mutable token has no expiry/refresh handling.
anthropicAuthTokenis a global singleton — if the OAuth token expires mid-session, all subsequent agent calls will fail with auth errors untilsetAnthropicAuthTokenis called again. Consider:
- Accepting a token-provider callback (e.g.,
() => Promise<string>) instead of a static string, so the caller can handle refresh transparently.- If staying with a static value, exposing a
clearAnthropicAuthToken()for logout/session teardown to avoid stale credentials lingering in memory.
127-127: Redundantas string | undefinedcast.
requestContext.get()already returnsstring | undefinedper the type signature on line 39. The same cast appears on line 216 as well.Suggested fix
- const cwd = requestContext.get("cwd") as string | undefined; + const cwd = requestContext.get("cwd");And similarly at line 216:
- const cwd = requestContext.get("cwd") as string | undefined; + const cwd = requestContext.get("cwd");
66-69: Module-level side effects:PostgresStoreandMastrainitialize eagerly on import.Both
storage(line 66) andmastra(line 232) establish infrastructure (Postgres connections, agent registry) at module load time. This means any file that imports anything from this module — even justsetAnthropicAuthToken— triggers a database connection attempt. This can cause unexpected failures in tests, tooling, or any context whereDATABASE_URLisn't configured.Consider lazy initialization (factory function or deferred singleton) if this module's exports are consumed in varied contexts.
Also applies to: 231-241
| // OAuth requires specific beta flags and Claude CLI identity headers. | ||
| // See: https://github.com/badlogic/pi-mono/blob/main/packages/ai/src/providers/anthropic.ts | ||
| return createAnthropic({ | ||
| authToken: anthropicAuthToken, | ||
| headers: { | ||
| "anthropic-beta": "claude-code-20250219,oauth-2025-04-20", | ||
| "user-agent": "claude-cli/2.1.2 (external, cli)", | ||
| "x-app": "cli", | ||
| }, | ||
| })(model); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Anthropic OAuth API required headers documentation
💡 Result:
Anthropic’s public Claude API (api.anthropic.com) does not document an “OAuth” flow; it documents API-key auth with required request headers: x-api-key, anthropic-version, and content-type: application/json. [1]
Required headers (Claude API):
x-api-key: <your key>[1]anthropic-version: 2023-06-01(required version header) [1][2]content-type: application/json[1]
Optional (only if you’re using a beta feature):
anthropic-beta: <beta_feature_name>[3]
Bearer token header (OAuth-like) — only documented for specific hosted offerings (e.g., Microsoft Foundry / Entra ID), not the standard Anthropic endpoint:
Authorization: Bearer <token>[4]
🌐 Web query:
Anthropic claude-code beta flag oauth 2025
💡 Result:
For Claude Code’s OAuth flow in 2025, Anthropic gated OAuth behind an API beta header. If that header is missing, the API can return 401 with “OAuth authentication is currently not supported.” (a failure mode reported for Claude Code/extension). [3]
What to include
- Send an
anthropic-betaheader that includes the OAuth beta, e.g.:anthropic-beta: oauth-2025-04-20(often alongside other betas in the same header) [1][2]
Why you might be seeing this
- A documented Claude Code regression (example: v2.0.22) removed the required
anthropic-betaheader for OAuth, breaking OAuth-authenticated calls until the header was added back (or users downgraded). [2]
Reference: how beta headers work
- Anthropic’s docs: set
anthropic-beta: <beta-feature-name>(comma-separated for multiple betas), where names commonly look likefeature-name-YYYY-MM-DD. [1]
Sources:
[1] Anthropic “Beta headers” documentation
[2] “Claude Code v2.0.22 OAuth Fix Patch” gist (documents missing header + shows oauth-2025-04-20)
[3] GitHub issue reporting OAuth tokens rejected with “OAuth authentication is currently not supported”
🌐 Web query:
Anthropic Terms of Service user-agent spoofing API client identity
💡 Result:
Anthropic’s public Commercial Terms of Service / Usage Policy don’t call out the HTTP User-Agent header specifically, but spoofing a user-agent to misrepresent who your API client is (or to evade enforcement) can fall under multiple binding restrictions:
- Commercial Terms (API customers): Anthropic incorporates the Usage Policy by reference and says you must cooperate with reasonable requests to support compliance, including requests “to verify [your] identity and use of the Services.” If you’re spoofing client identity to frustrate attribution/enforcement, that conflicts with this framework. [1]
- Usage Policy (applies to API use): Prohibits “gain[ing] unauthorized access … through technical attacks or social engineering” and “bypass[ing] security controls,” plus broader prohibitions on deceptive impersonation / fake personas and other abusive behavior (e.g., circumventing bans/guardrails). Using a spoofed
User-Agentto impersonate another app/service (or to evade rate limits, blocks, monitoring, or attribution) is most likely to be treated as policy-violating circumvention/deception. [2] - If the spoofing is “pretend to be Anthropic” (e.g.,
ClaudeBot,Claude-User,Claude-SearchBot): Anthropic publicly documents these crawler identities and frames them as part of “transparen[cy] and choice” for website operators; masquerading as them is plainly deceptive (and may also create legal issues with third parties). [3]
Practical takeaway: Setting any User-Agent string isn’t inherently forbidden, but using it to misrepresent your identity/affiliation or to evade controls is risky and likely non-compliant with Anthropic’s Terms/Usage Policy. [1][2]
Sources
[1] Anthropic – Commercial Terms of Service (effective June 17, 2025) https://www.anthropic.com/legal/commercial-terms
[2] Anthropic – Usage Policy (effective Sept 15, 2025) https://www.anthropic.com/legal/aup
[3] Anthropic Help Center – Web crawling bots / how to block (ClaudeBot / Claude-User / Claude-SearchBot) https://support.anthropic.com/en/articles/8896518-does-anthropic-crawl-data-from-the-web-and-how-can-site-owners-block-the-crawler
Spoofed user-agent header violates Anthropic's Usage Policy.
The hard-coded "user-agent": "claude-cli/2.1.2" and "x-app": "cli" headers misrepresent the client identity as the official Claude CLI. Anthropic's Usage Policy prohibits "deceptive impersonation" and "circumventing security controls or enforcement," and explicitly states that spoofing identity to evade rate limits, monitoring, or attribution is non-compliant.
The beta headers (anthropic-beta: claude-code-20250219,oauth-2025-04-20) are officially documented as required for OAuth support in Claude Code 2025 and are not artifacts; they will be maintained as long as those features are in beta. However, the real issue is the misrepresented user-agent.
Use a truthful user-agent string (e.g., "superagent/1.0") to accurately identify this client application.
🤖 Prompt for AI Agents
In `@packages/agent/src/superagent.ts` around lines 50 - 59, The hard-coded
deceptive client identity is set in the createAnthropic call: update the headers
passed to createAnthropic (where anthropicAuthToken and model are used) to stop
impersonating the official Claude CLI; keep the required "anthropic-beta" header
but replace "user-agent": "claude-cli/2.1.2 (external, cli)" and "x-app": "cli"
with truthful identifiers (for example "user-agent": "superagent/1.0" and
"x-app": "superagent" or remove x-app entirely), so the client accurately
represents itself while preserving OAuth beta flags.
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`:
- Line 50: The UI toggle state thinkingEnabled is not being forwarded to the
backend: update the payload passed to triggerAgent.mutate inside handleSend to
include the thinkingEnabled boolean (so the agent receives the toggle), and add
thinkingEnabled to the useCallback dependency array for handleSend to avoid
stale closures; refer to the thinkingEnabled/setThinkingEnabled state, the
ThinkingToggle UI, the handleSend function, and the triggerAgent.mutate call
when making these changes.
- Around line 195-227: handleAnswer currently updates local message state but
conditionally sends answerQuestionMutation based on the mutable pendingApproval,
which can race and cause the mutation to be skipped; fix by capturing
pendingApproval into a local constant at the start of handleAnswer (e.g., const
approval = pendingApproval) then use that captured approval for the mutation and
clearing (answerQuestionMutation.mutate with approval.runId and
setPendingApproval(null)), or split responsibilities so the state update always
runs and the mutation is triggered from a stabilized ref/closure separate from
pendingApproval — change references to pendingApproval in handleAnswer to use
the captured variable to avoid the race.
- Around line 96-101: The onError for triggerAgent
(electronTrpc.aiChat.superagent.useMutation) only logs to console and sets
setIsStreaming(false); update the onError handler to also call setError(...)
with a user-facing message (e.g., err.message || String(err) or a fallback like
"Failed to trigger agent") so users see the failure; ensure you still call
setIsStreaming(false) and preserve existing console.error for debugging.
In `@packages/agent/package.json`:
- Around line 17-27: Update the invalid dependency versions in package.json:
change "@ai-sdk/anthropic" from "3.0.43" to "3.0.41", change "@mastra/ai-sdk"
from "1.0.4" to the published "0.3.3", and change "@mastra/memory" from "1.2.0"
to the stable "1.3.0" so npm install uses existing registry versions; leave the
other entries (e.g., "@tavily/core", "@mastra/pg", "linkedom") unchanged.
In `@packages/agent/src/superagent.ts`:
- Around line 24-28: The module-level mutable anthropicAuthToken and setter
setAnthropicAuthToken must be removed and the token passed per-invocation via
the existing requestContext (the same way modelId and cwd are handled); update
any functions in this module that currently read anthropicAuthToken to instead
accept or read a token field from requestContext, propagate the token from
callers that currently call setAnthropicAuthToken, and delete or deprecate
setAnthropicAuthToken to avoid global state. Ensure all references to
anthropicAuthToken are replaced with requestContext.<token> (or a named field
like requestContext.anthropicAuthToken) so concurrent sessions use their own
tokens.
In `@packages/agent/src/tools/ask-user-question.ts`:
- Around line 54-58: The current catch and final fallback both return { answers:
{} } which is indistinguishable from a successful empty response; change the
behavior in the askUserQuestion flow so errors are signaled explicitly (e.g.,
return a sentinel/result object with an error flag or message instead of
silently returning { answers: {} } or rethrow the error). Specifically, modify
the catch block around the toolAnswers parsing and the final return to produce a
distinguishable shape (for example include error: true / errorMessage or
success: false) or throw an error, and update callers of askUserQuestion (and
any code inspecting toolAnswers/answers) to handle that new sentinel/result so
the agent can retry or inform the user. Ensure the unique symbols touched are
the catch where toolAnswers is parsed and the final return that currently yields
{ answers: {} }.
In `@packages/agent/src/tools/web-fetch.ts`:
- Around line 14-19: The inputSchema defines prompt (z.string().optional()) but
the execute function never reads it; update the execute implementation in
web-fetch.ts to consume the prompt parameter from the parsed input and pass it
into the content-extraction/summarisation flow (e.g., forward to the summariser
or use to filter extracted text) or remove prompt from inputSchema if you intend
not to support it; specifically, modify the execute function that uses the
parsed input object to reference input.prompt and incorporate it into the
extraction/summary step (or delete the prompt field from inputSchema) so user
intent is not silently discarded.
- Around line 27-35: The fetch call using input.url in web-fetch.ts allows SSRF
because it doesn't validate scheme or the resolved IP; update the code around
the fetch invocation (the code referencing input.url and AbortSignal.timeout)
to: 1) allow only safe schemes (e.g., require https, optionally allow http) by
validating the URL.protocol; 2) perform a DNS resolution of the hostname and
validate each resolved IP against a denylist of private/reserved/link-local
ranges (RFC1918, localhost, 169.254.0.0/16, IPv6 link-local, etc.) and block if
any match; and 3) reject the request with a clear error instead of calling fetch
when validation fails. Ensure the hostname/IP check runs before calling fetch so
only public routable addresses are fetched.
- Around line 60-66: The truncation uses UTF-16 characters but bytes were
measured in UTF-8, so change the logic in the block around
TextEncoder/bytes/MAX_CONTENT_BYTES to operate on encoded bytes: encode content
to a Uint8Array with TextEncoder, if encoded.length > MAX_CONTENT_BYTES take
encoded.slice(0, MAX_CONTENT_BYTES), decode that byte slice back to a string
with TextDecoder (so truncated content is byte-accurate), then set content to
that decoded string plus the existing “[Content truncated — ${bytes} bytes
total, showing first ${MAX_CONTENT_BYTES}]” message; update references to bytes
to use encoded.length and use TextEncoder/TextDecoder instead of string.slice to
avoid cutting multi-byte characters.
🧹 Nitpick comments (9)
packages/agent/src/tools/ask-user-question.ts (1)
50-56: Unsafe cast of parsed JSON — validate with Zod instead ofas.
JSON.parse(raw) as Record<string, string>trusts the shape blindly. If the injectedtoolAnswersvalue is malformed (e.g., nested objects, arrays, or numeric values), the tool will return data that violates its ownoutputSchemacontract without any indication of failure.Since you already have the exact Zod shape on Line 43, reuse it for runtime validation:
♻️ Proposed fix
execute: async (_input, context) => { + const answersSchema = z.record(z.string(), z.string()); const raw = context?.requestContext?.get("toolAnswers") as | string | undefined; if (raw) { try { - const answers = JSON.parse(raw) as Record<string, string>; - return { answers }; + const parsed = answersSchema.safeParse(JSON.parse(raw)); + if (parsed.success) { + return { answers: parsed.data }; + } + return { answers: {} }; } catch { return { answers: {} }; } } return { answers: {} }; },apps/desktop/src/lib/trpc/routers/ai-chat/index.ts (4)
316-336: Remove verbose debug logging from production code.The loop that logs tool-part keys and JSON-stringified content for the first 3 messages is development scaffolding that shouldn't ship. It runs on every
getMessagescall, adding unnecessary I/O and potentially logging sensitive prompt/tool data.Proposed fix
.query(async ({ input }) => { const result = await memory.recall({ threadId: input.threadId, }); - // Convert Mastra DB messages → AI SDK V5 UIMessage format - // This normalizes tool invocations with proper toolName, toolCallId, etc. const uiMessages = toAISdkV5Messages(result.messages); - // Debug: log tool parts to verify shape - for (const msg of uiMessages.slice(0, 3)) { - const parts = (msg as Record<string, unknown>).parts as - | Array<Record<string, unknown>> - | undefined; - if (parts) { - for (const p of parts) { - if (String(p.type ?? "").startsWith("tool-")) { - console.log( - "[getMessages] V5 tool part keys:", - Object.keys(p), - "type:", - p.type, - ); - console.log( - "[getMessages] V5 tool part:", - JSON.stringify(p, null, 2).slice(0, 500), - ); - } - } - } - } return uiMessages; }),
558-559: Operator precedence makes this expression misleading.
projectContext + fileMentionContext || undefinedparses as(projectContext + fileMentionContext) || undefined. It happens to work because concatenating two empty strings yields""(falsy), but the intent is obscure and fragile. A reader might expectprojectContext + (fileMentionContext || undefined), which would produce"<context>undefined".Proposed fix — make the intent explicit
- const contextInstructions = - projectContext + fileMentionContext || undefined; + const combined = projectContext + fileMentionContext; + const contextInstructions = combined || undefined;
621-636: Excessive per-chunk logging will flood the console in production.The conditional on lines 624-631 logs for every
text-delta, everytool*chunk, everyfinish/start/step-*event, and the first 5 chunks unconditionally. For a typical long response, nearly every chunk matches at least one of these conditions, making this effectively unconditional logging of the entire stream. Consider gating behind a debug flag or removing entirely.
832-923:answerQuestionduplicates nearly all ofapproveToolCall's resume-stream logic.The body of
answerQuestion(lines 855-920) is an almost line-for-line copy of the fire-and-forget block inapproveToolCall(lines 746-827): buildctxEntries, inject pending answers, createRequestContext, callapproveToolCallon the agent, iteratefullStream, detect re-suspension, emit done/error. The only difference is thatanswerQuestionalways approves and pre-stores answers on line 849.Extract a shared helper (e.g.,
resumeStream(sessionId, runId, approved, extraCtxEntries?)) to eliminate the duplication and ensure future fixes (like the missingAbortControllerregistration flagged earlier) are applied in one place.apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx (2)
158-164:handleModelSelectandhandlePermissionModeSelectare trivial wrappers — consider passing setters directly.These
useCallbackwrappers with empty dependency arrays are functionally equivalent to passingsetSelectedModelandsetPermissionModedirectly. React state setters are already stable references.
46-46: Component is growing large — consider extracting message rendering logic.
ChatInterfacehandles state management, mutation orchestration, approval flows, and all rendering. The message list rendering block (lines 247–288) and the prompt input section (lines 302–361) are good candidates for extraction into sub-components, improving readability and testability.Also applies to: 236-364
packages/agent/src/tools/web-fetch.ts (1)
48-58: Wrap body-read and parsing in a try/catch to return a graceful error.
response.text(),parseHTML, andReadability.parse()can all throw on malformed input or encoding issues. An unhandled exception here would bubble up as a raw tool failure. Catching and returning a structured error (similar to the!response.okpath) keeps the agent loop stable.Proposed fix
- const rawHtml = await response.text(); - let content: string; - - if (contentType.includes("text/html") || contentType.includes("xhtml")) { - const { document } = parseHTML(rawHtml); - const reader = new Readability(document); - const article = reader.parse(); - content = article?.textContent?.trim() ?? rawHtml; - } else { - content = rawHtml; - } + let rawHtml: string; + try { + rawHtml = await response.text(); + } catch { + return { content: "Failed to read response body", bytes: 0, status_code: statusCode }; + } + + let content: string; + try { + if (contentType.includes("text/html") || contentType.includes("xhtml")) { + const { document } = parseHTML(rawHtml); + const reader = new Readability(document); + const article = reader.parse(); + content = article?.textContent?.trim() ?? rawHtml; + } else { + content = rawHtml; + } + } catch { + content = rawHtml; + }packages/agent/src/tools/web-search.ts (1)
36-47: Consider wrapping the Tavily API call in a try/catch for better error messages.If the Tavily API call fails (network error, rate limit, invalid response), the raw error will propagate without context. A try/catch with a descriptive re-throw would improve debuggability for end users of the agent.
♻️ Suggested improvement
const client = tavily({ apiKey }); - const response = await client.search(input.query, { - maxResults: input.maxResults, - }); + let response; + try { + response = await client.search(context.query, { + maxResults: context.maxResults, + }); + } catch (error) { + throw new Error( + `Web search failed for query "${context.query}": ${error instanceof Error ? error.message : String(error)}`, + ); + }
| const [selectedModel, setSelectedModel] = | ||
| useState<ModelOption>(DEFAULT_MODEL); | ||
| const [modelSelectorOpen, setModelSelectorOpen] = useState(false); | ||
| const [thinkingEnabled, setThinkingEnabled] = useState(false); |
There was a problem hiding this comment.
thinkingEnabled is tracked in state but never sent to the backend.
The toggle updates local state (line 50), and the UI renders a ThinkingToggle (line 324), but handleSend at lines 139–145 doesn't include thinkingEnabled in the triggerAgent.mutate(...) payload. The feature toggle has no effect on agent behavior.
Proposed fix
triggerAgent.mutate({
sessionId,
text,
modelId: selectedModel.id,
cwd,
permissionMode,
+ thinkingEnabled,
});And add thinkingEnabled to the useCallback dependency array.
Also applies to: 139-145
🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`
at line 50, The UI toggle state thinkingEnabled is not being forwarded to the
backend: update the payload passed to triggerAgent.mutate inside handleSend to
include the thinkingEnabled boolean (so the agent receives the toggle), and add
thinkingEnabled to the useCallback dependency array for handleSend to avoid
stale closures; refer to the thinkingEnabled/setThinkingEnabled state, the
ThinkingToggle UI, the handleSend function, and the triggerAgent.mutate call
when making these changes.
| const triggerAgent = electronTrpc.aiChat.superagent.useMutation({ | ||
| onError: (err) => { | ||
| console.error("[chat] Agent trigger failed:", err); | ||
| setIsStreaming(false); | ||
| }, | ||
| [sessionId], | ||
| ); | ||
|
|
||
| const { allMessages } = useClaudeCodeHistory({ | ||
| sessionId, | ||
| liveMessages: messages, | ||
| onRename: handleRename, | ||
| }); |
There was a problem hiding this comment.
triggerAgent error handler doesn't set the error message for the user.
The onError callback logs to console but doesn't call setError(...), so if the mutation fails (network issue, server error), the user sees no feedback — the UI just silently stops streaming.
Proposed fix
const triggerAgent = electronTrpc.aiChat.superagent.useMutation({
onError: (err) => {
console.error("[chat] Agent trigger failed:", err);
+ setError(err.message ?? "Failed to start agent");
setIsStreaming(false);
},
});🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`
around lines 96 - 101, The onError for triggerAgent
(electronTrpc.aiChat.superagent.useMutation) only logs to console and sets
setIsStreaming(false); update the onError handler to also call setError(...)
with a user-facing message (e.g., err.message || String(err) or a fallback like
"Failed to trigger agent") so users see the failure; ensure you still call
setIsStreaming(false) and preserve existing console.error for debugging.
| const handleAnswer = useCallback( | ||
| (toolCallId: string, answers: Record<string, string>) => { | ||
| // Update local state to mark the tool call as answered | ||
| setMessages((prev) => | ||
| prev.map((msg) => { | ||
| if (msg.role !== "assistant") return msg; | ||
| return { | ||
| ...msg, | ||
| parts: msg.parts.map((part) => | ||
| part.type === "tool-call" && part.toolCallId === toolCallId | ||
| ? { | ||
| ...part, | ||
| status: "done" as const, | ||
| result: { answers }, | ||
| } | ||
| : part, | ||
| ), | ||
| }; | ||
| }), | ||
| ); | ||
|
|
||
| // Resume the agent stream with answers injected into RequestContext | ||
| if (pendingApproval) { | ||
| answerQuestionMutation.mutate({ | ||
| sessionId, | ||
| runId: pendingApproval.runId, | ||
| answers, | ||
| }); | ||
| setPendingApproval(null); | ||
| } | ||
| }, | ||
| [interruptAgent, sessionId, stop], | ||
| [pendingApproval, answerQuestionMutation, sessionId], | ||
| ); |
There was a problem hiding this comment.
handleAnswer couples the question-answer flow tightly to pendingApproval.
The local state update (lines 198–214) uses toolCallId to find the right part — good. But the mutation dispatch (lines 217–224) depends on pendingApproval being non-null, and it's cleared immediately. If handleAnswer is called when pendingApproval has already been cleared (e.g., race with a stream event), the mutation silently doesn't fire despite the local state being updated, leaving the agent stuck.
Consider capturing pendingApproval at the start of the function or separating the two concerns:
Proposed fix
const handleAnswer = useCallback(
(toolCallId: string, answers: Record<string, string>) => {
+ const approval = pendingApproval;
// Update local state to mark the tool call as answered
setMessages((prev) =>
prev.map((msg) => { /* ... */ }),
);
- if (pendingApproval) {
+ if (approval) {
answerQuestionMutation.mutate({
sessionId,
- runId: pendingApproval.runId,
+ runId: approval.runId,
answers,
});
setPendingApproval(null);
}
},
[pendingApproval, answerQuestionMutation, sessionId],
);🤖 Prompt for AI Agents
In
`@apps/desktop/src/renderer/screens/main/components/WorkspaceView/ContentView/TabsContent/TabView/ChatPane/ChatInterface/ChatInterface.tsx`
around lines 195 - 227, handleAnswer currently updates local message state but
conditionally sends answerQuestionMutation based on the mutable pendingApproval,
which can race and cause the mutation to be skipped; fix by capturing
pendingApproval into a local constant at the start of handleAnswer (e.g., const
approval = pendingApproval) then use that captured approval for the mutation and
clearing (answerQuestionMutation.mutate with approval.runId and
setPendingApproval(null)), or split responsibilities so the state update always
runs and the mutation is triggered from a stabilized ref/closure separate from
pendingApproval — change references to pendingApproval in handleAnswer to use
the captured variable to avoid the race.
| "@ai-sdk/anthropic": "^3.0.43", | ||
| "@anthropic-ai/claude-agent-sdk": "^0.2.38", | ||
| "@mastra/ai-sdk": "^1.0.4", | ||
| "@mastra/core": "^1.3.0", | ||
| "@mastra/memory": "^1.2.0", | ||
| "@mastra/pg": "^1.3.0", | ||
| "@mozilla/readability": "^0.6.0", | ||
| "@superset/durable-session": "workspace:*", | ||
| "@tanstack/ai": "^0.3.0", | ||
| "@tavily/core": "^0.7.1", | ||
| "linkedom": "^0.18.12", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify package versions exist on npm registry
packages=(
"@ai-sdk/anthropic@3.0.43"
"@mastra/ai-sdk@1.0.4"
"@mastra/core@1.3.0"
"@mastra/memory@1.2.0"
"@mastra/pg@1.3.0"
"@tavily/core@0.7.1"
"linkedom@0.18.12"
)
for pkg in "${packages[@]}"; do
echo "=== Checking $pkg ==="
# Extract package name and version
if [[ $pkg == @* ]]; then
# Handle scoped packages
name=$(echo "$pkg" | rev | cut -d'@' -f2- | rev)
version=$(echo "$pkg" | rev | cut -d'@' -f1 | rev)
else
name=$(echo "$pkg" | cut -d'@' -f1)
version=$(echo "$pkg" | cut -d'@' -f2)
fi
# Check if version exists on npm
response=$(curl -s "https://registry.npmjs.org/${name}/${version}")
if echo "$response" | grep -q '"version"'; then
echo "✓ Version exists"
else
echo "✗ Version not found or error"
echo "Response: $(echo "$response" | head -c 200)"
fi
doneRepository: superset-sh/superset
Length of output: 1639
🌐 Web query:
@ai-sdk/anthropic 3.0.43 security vulnerability
💡 Result:
I couldn’t find any published security vulnerability/advisory specifically affecting @ai-sdk/anthropic v3.0.43.
@ai-sdk/anthropicv3.0.43 doesn’t appear in public trackers I checked (e.g., Snyk’s version list shows 3.0.41 but not 3.0.43), so the “3.0.43” version may be a typo, unpublished, or released after the tracker snapshot. [1]- The upstream repo
vercel/aihas no published GitHub Security Advisories at all (at least at the time of the snapshot). [2] - Multiple package scanners/trackers show no known/direct vulnerabilities for
@ai-sdk/anthropicversions they list. [1], [3]
Note: there is a CVE for the ai npm package (not @ai-sdk/anthropic) about bypassing filetype whitelists in uploads, affecting ai versions < 5.0.52 (and some betas). [4]
If you share the advisory/CVE/GHSA link (or the tool output flagging 3.0.43), I can map it precisely to affected versions and remediation.
Sources: [1] [2] [3] [4]
🌐 Web query:
@mastra/ai-sdk @mastra/core security vulnerabilities 2025 2026
💡 Result:
Findings (2025–2026)
@mastra/core: As of Feb 2026, Snyk lists no known direct vulnerabilities for@mastra/core(latest shown: 1.3.0). [1]@mastra/ai-sdk: I did not find any public CVEs/advisories specifically for@mastra/ai-sdk; Aikido Intel reports 0 vulnerabilities for the package (latest shown: 0.3.3). [2]
Related Mastra advisory (not these two packages, but in the ecosystem)
- CVE-2025-61685 / GHSA-xh92-rqrq-227v: A directory traversal / directory listing disclosure affecting
@mastra/mcp-docs-server(patched in 0.17.0)—this is not@mastra/coreor@mastra/ai-sdk, but it may matter if your project uses that MCP server. [3][4]
Sources: Snyk package page for @mastra/core [1], Aikido Intel package page for @mastra/ai-sdk [2], NVD CVE entry [3], GitHub Security Advisory [4].
🌐 Web query:
linkedom 0.18.12 security vulnerability
💡 Result:
No public security vulnerability specific to linkedom v0.18.12 appears to be currently disclosed:
- GitHub (WebReflection/linkedom): the project shows no published security advisories. [1]
- Snyk (npm linkedom): reports 0 direct vulnerabilities for v0.18.12 and lists 0.18.12 as the latest non‑vulnerable version (published 21 Aug 2025). [2]
- Aikido Intel: reports no vulnerabilities found for the package. [3]
If you’re seeing an alert in a specific scanner, it’s likely an indirect/dependency or heuristic/supply-chain flag—share the exact finding (CVE/GHSA/OSV ID or scanner output) and I can validate it against primary sources.
Sources: [1] [2] [3]
🏁 Script executed:
#!/bin/bash
# More robust check for npm packages using simpler parsing
echo "=== Checking `@tavily/core` latest versions ==="
curl -s "https://registry.npmjs.org/@tavily/core" | jq '.versions | keys | .[-5:]' 2>/dev/null || echo "Could not fetch"
echo ""
echo "=== Checking `@mastra/pg` latest versions ==="
curl -s "https://registry.npmjs.org/@mastra/pg" | jq '.versions | keys | .[-5:]' 2>/dev/null || echo "Could not fetch"
echo ""
echo "=== Checking `@mastra/memory` latest versions ==="
curl -s "https://registry.npmjs.org/@mastra/memory" | jq '.versions | keys | .[-5:]' 2>/dev/null || echo "Could not fetch"
echo ""
echo "=== Checking `@ai-sdk/anthropic` version 3.0.41 vs 3.0.43 ==="
curl -s "https://registry.npmjs.org/@ai-sdk/anthropic" | jq '.versions | keys | map(select(startswith("3.0"))) | sort_by(split(".") | map(tonumber))' 2>/dev/null || echo "Could not fetch"Repository: superset-sh/superset
Length of output: 510
Fix package version references that do not exist on npm registry.
Three dependencies reference versions that are not published:
@ai-sdk/anthropic@3.0.43does not exist; latest available is3.0.41@mastra/ai-sdk@1.0.4does not exist; only0.3.3is available@mastra/memory@1.2.0does not exist as a stable release; only alpha versions are published, with latest stable being1.3.0
Update these to valid published versions to prevent npm install failures. The other dependencies (@tavily/core@0.7.1, @mastra/pg@1.3.0, and linkedom@0.18.12) exist and have no known security vulnerabilities.
🤖 Prompt for AI Agents
In `@packages/agent/package.json` around lines 17 - 27, Update the invalid
dependency versions in package.json: change "@ai-sdk/anthropic" from "3.0.43" to
"3.0.41", change "@mastra/ai-sdk" from "1.0.4" to the published "0.3.3", and
change "@mastra/memory" from "1.2.0" to the stable "1.3.0" so npm install uses
existing registry versions; leave the other entries (e.g., "@tavily/core",
"@mastra/pg", "linkedom") unchanged.
| let anthropicAuthToken: string | null = null; | ||
|
|
||
| export function setAnthropicAuthToken(token: string) { | ||
| anthropicAuthToken = token; | ||
| } |
There was a problem hiding this comment.
Global mutable anthropicAuthToken is shared across all concurrent sessions.
anthropicAuthToken is a module-level singleton. If multiple users or sessions call setAnthropicAuthToken with different OAuth tokens, they silently overwrite each other — the last writer wins and all subsequent agent invocations (including those belonging to other sessions) use that token. This is both a correctness bug (wrong credentials) and a security issue (token leakage across sessions).
Pass the token per-invocation via requestContext instead of relying on module-level state, which is already the pattern used for modelId and cwd.
Suggested approach
-let anthropicAuthToken: string | null = null;
-
-export function setAnthropicAuthToken(token: string) {
- anthropicAuthToken = token;
-}
-
...
function resolveModel({
requestContext,
}: {
requestContext: { get: (key: string) => string | undefined };
}) {
const modelId =
requestContext.get("modelId") ?? "anthropic/claude-sonnet-4-5";
+ const authToken = requestContext.get("anthropicAuthToken");
- if (anthropicAuthToken) {
+ if (authToken) {
const slashIdx = modelId.indexOf("/");
const provider = slashIdx > -1 ? modelId.slice(0, slashIdx) : "anthropic";
const model = slashIdx > -1 ? modelId.slice(slashIdx + 1) : modelId;
if (provider === "anthropic") {
return createAnthropic({
- authToken: anthropicAuthToken,
+ authToken,
headers: { ... },
})(model);
}
}
return modelId;
}The caller that currently calls setAnthropicAuthToken(token) would instead pass the token through requestContext when invoking the agent.
🤖 Prompt for AI Agents
In `@packages/agent/src/superagent.ts` around lines 24 - 28, The module-level
mutable anthropicAuthToken and setter setAnthropicAuthToken must be removed and
the token passed per-invocation via the existing requestContext (the same way
modelId and cwd are handled); update any functions in this module that currently
read anthropicAuthToken to instead accept or read a token field from
requestContext, propagate the token from callers that currently call
setAnthropicAuthToken, and delete or deprecate setAnthropicAuthToken to avoid
global state. Ensure all references to anthropicAuthToken are replaced with
requestContext.<token> (or a named field like requestContext.anthropicAuthToken)
so concurrent sessions use their own tokens.
| } catch { | ||
| return { answers: {} }; | ||
| } | ||
| } | ||
| return { answers: {} }; |
There was a problem hiding this comment.
Silent empty-answers fallback may mislead the agent.
Both error paths (parse failure on Line 54 and missing toolAnswers on Line 58) return { answers: {} }, which is indistinguishable from a successful result with no answers. The agent has no signal that something went wrong and may proceed as if the user declined to answer.
Consider either logging/throwing on the error path or returning a distinguishable sentinel so the agent can retry or inform the user.
🤖 Prompt for AI Agents
In `@packages/agent/src/tools/ask-user-question.ts` around lines 54 - 58, The
current catch and final fallback both return { answers: {} } which is
indistinguishable from a successful empty response; change the behavior in the
askUserQuestion flow so errors are signaled explicitly (e.g., return a
sentinel/result object with an error flag or message instead of silently
returning { answers: {} } or rethrow the error). Specifically, modify the catch
block around the toolAnswers parsing and the final return to produce a
distinguishable shape (for example include error: true / errorMessage or
success: false) or throw an error, and update callers of askUserQuestion (and
any code inspecting toolAnswers/answers) to handle that new sentinel/result so
the agent can retry or inform the user. Ensure the unique symbols touched are
the catch where toolAnswers is parsed and the final return that currently yields
{ answers: {} }.
| prompt: z | ||
| .string() | ||
| .optional() | ||
| .describe( | ||
| "Optional prompt describing what information to look for on the page", | ||
| ), |
There was a problem hiding this comment.
prompt is declared in the input schema but never used in execute.
The agent (or caller) can supply a prompt describing what to look for, but the execute function never reads it. This silently discards user intent. Either remove the field from inputSchema or incorporate it into the content-extraction logic (e.g., pass it to a summarisation step or use it to filter content).
🤖 Prompt for AI Agents
In `@packages/agent/src/tools/web-fetch.ts` around lines 14 - 19, The inputSchema
defines prompt (z.string().optional()) but the execute function never reads it;
update the execute implementation in web-fetch.ts to consume the prompt
parameter from the parsed input and pass it into the
content-extraction/summarisation flow (e.g., forward to the summariser or use to
filter extracted text) or remove prompt from inputSchema if you intend not to
support it; specifically, modify the execute function that uses the parsed input
object to reference input.prompt and incorporate it into the extraction/summary
step (or delete the prompt field from inputSchema) so user intent is not
silently discarded.
| const response = await fetch(input.url, { | ||
| headers: { | ||
| "User-Agent": | ||
| "Mozilla/5.0 (compatible; SupersetAgent/1.0; +https://superset.sh)", | ||
| Accept: "text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8", | ||
| }, | ||
| redirect: "follow", | ||
| signal: AbortSignal.timeout(15_000), | ||
| }); |
There was a problem hiding this comment.
SSRF risk: no validation that the URL points to a public host.
The tool fetches any URL the agent provides. A prompt-injection or a malicious link could direct it to internal network addresses (e.g., http://169.254.169.254/... for cloud metadata, http://localhost:..., private RFC-1918 ranges). Consider:
- Blocking private/reserved IP ranges and link-local addresses before fetching.
- Restricting the scheme to
https(and optionallyhttp). - Resolving the hostname and validating the resolved IP before connecting.
🤖 Prompt for AI Agents
In `@packages/agent/src/tools/web-fetch.ts` around lines 27 - 35, The fetch call
using input.url in web-fetch.ts allows SSRF because it doesn't validate scheme
or the resolved IP; update the code around the fetch invocation (the code
referencing input.url and AbortSignal.timeout) to: 1) allow only safe schemes
(e.g., require https, optionally allow http) by validating the URL.protocol; 2)
perform a DNS resolution of the hostname and validate each resolved IP against a
denylist of private/reserved/link-local ranges (RFC1918, localhost,
169.254.0.0/16, IPv6 link-local, etc.) and block if any match; and 3) reject the
request with a clear error instead of calling fetch when validation fails.
Ensure the hostname/IP check runs before calling fetch so only public routable
addresses are fetched.
| const bytes = new TextEncoder().encode(content).length; | ||
|
|
||
| if (bytes > MAX_CONTENT_BYTES) { | ||
| content = | ||
| content.slice(0, MAX_CONTENT_BYTES) + | ||
| `\n\n[Content truncated — ${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]`; | ||
| } |
There was a problem hiding this comment.
Character-based slice doesn't honour the byte-based limit.
bytes is measured in UTF-8 bytes, but content.slice(0, MAX_CONTENT_BYTES) truncates by UTF-16 code units. For multi-byte content (CJK, emoji, etc.), 50 000 characters can encode to well over 50 000 bytes, so the truncation under-cuts for ASCII but overshoots for non-ASCII text.
If byte-accurate truncation matters, encode first, then truncate the byte array:
Proposed fix
- const bytes = new TextEncoder().encode(content).length;
-
- if (bytes > MAX_CONTENT_BYTES) {
- content =
- content.slice(0, MAX_CONTENT_BYTES) +
- `\n\n[Content truncated — ${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]`;
- }
+ const encoded = new TextEncoder().encode(content);
+ const bytes = encoded.length;
+
+ if (bytes > MAX_CONTENT_BYTES) {
+ content =
+ new TextDecoder().decode(encoded.slice(0, MAX_CONTENT_BYTES)) +
+ `\n\n[Content truncated — ${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]`;
+ }📝 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 bytes = new TextEncoder().encode(content).length; | |
| if (bytes > MAX_CONTENT_BYTES) { | |
| content = | |
| content.slice(0, MAX_CONTENT_BYTES) + | |
| `\n\n[Content truncated — ${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]`; | |
| } | |
| const encoded = new TextEncoder().encode(content); | |
| const bytes = encoded.length; | |
| if (bytes > MAX_CONTENT_BYTES) { | |
| content = | |
| new TextDecoder().decode(encoded.slice(0, MAX_CONTENT_BYTES)) + | |
| `\n\n[Content truncated — ${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]`; | |
| } |
🤖 Prompt for AI Agents
In `@packages/agent/src/tools/web-fetch.ts` around lines 60 - 66, The truncation
uses UTF-16 characters but bytes were measured in UTF-8, so change the logic in
the block around TextEncoder/bytes/MAX_CONTENT_BYTES to operate on encoded
bytes: encode content to a Uint8Array with TextEncoder, if encoded.length >
MAX_CONTENT_BYTES take encoded.slice(0, MAX_CONTENT_BYTES), decode that byte
slice back to a string with TextDecoder (so truncated content is byte-accurate),
then set content to that decoded string plus the existing “[Content truncated —
${bytes} bytes total, showing first ${MAX_CONTENT_BYTES}]” message; update
references to bytes to use encoded.length and use TextEncoder/TextDecoder
instead of string.slice to avoid cutting multi-byte characters.
2 participants
Description
Related Issues
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Summary by CodeRabbit
New Features
Improvements