feat: add subagents #2928
feat: add subagents #2928
Conversation
… agent type handling
refactor: toolset-lookup/registry refactor: useChat to user agenttype.root by default refactor: tools to use tools for agenttype feat: onlook-chat: implementation of AbstractChat from ai sdk feat: SubAgent tool arch allowing subagents to call tools on the client within their own threads feat: Example SubAgent -- allows to list files todo: add context to subagents todo: parent child thread relationship
|
@samschooler is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds AgentType (ROOT/USER) across models, DB, API and client; introduces UserAgent and UserAgentTool plus sub-agent tooling core; refactors tool registry to be agent-aware; makes initModel and provider helpers synchronous; adds OnlookChat state/manager and updates streaming telemetry/metadata. Changes
Sequence Diagram(s)sequenceDiagram
participant UI
participant ClientHook as useChat
participant API as ChatRoute
participant AgentFactory
participant RootAgent
participant UserAgent
participant Streamer as AgentStreamer
participant ToolRegistry
participant ToolInstance
UI->>ClientHook: send chat (agentType + messages)
ClientHook->>API: POST /chat (validated body includes agentType)
API->>AgentFactory: instantiate agent for agentType
AgentFactory-->>RootAgent: create RootAgent (if ROOT)
AgentFactory-->>UserAgent: create UserAgent (if USER)
API->>Streamer: streamText(agentInstance, messages)
Streamer->>ToolRegistry: getAvailableTools(agentType, chatType)
Streamer->>ToolInstance: tool.handle(..., getOnToolCall)
ToolInstance->>Streamer: invoke nested on-tool calls via getOnToolCall
Streamer->>UI: stream messages + metadata (agentType, usage, finishReason)
sequenceDiagram
participant Caller
participant initModel
rect rgb(240,248,255)
Caller->>initModel: initModel(opts)
initModel-->>Caller: ModelConfig (synchronous return)
end
note right of Caller: Previously returned a Promise and was awaited
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🧰 Additional context used📓 Path-based instructions (4)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
{apps,packages}/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/web/client/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
apps/web/client/src/app/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧬 Code graph analysis (2)packages/ai/src/agents/tools/base.ts (4)
apps/web/client/src/app/api/chat/helpers/stream.ts (1)
🔇 Additional comments (2)
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 |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
apps/web/client/src/server/api/routers/chat/suggestion.ts (1)
21-44: Pass providerOptions from initModel to generateObject.You compute providerOptions (e.g., OpenRouter transforms, Anthropic cache control) but don’t pass them, so those settings are ignored.
Apply this diff:
- const { model, headers } = initModel({ + const { model, headers, providerOptions } = initModel({ provider: LLMProvider.OPENROUTER, model: OPENROUTER_MODELS.OPEN_AI_GPT_5_NANO, }); const { object } = await generateObject({ model, headers, + providerOptions, schema: ChatSuggestionsSchema, messages: [apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (1)
140-145: Pass agentType on edited-message regenerate requestsThe new chat pipeline expects
agentTypeon every regenerate call. ThesendMessagepath was updated, but the edit flow still omits it, so edited messages now retry without an agent type—causing the server-side schema/agent lookup to fail. Please include the agent type here as well.void regenerate({ body: { chatType, conversationId, + agentType, }, });
🧹 Nitpick comments (4)
packages/ai/src/agents/tool-lookup.ts (1)
4-25: Make tool arrays readonly to prevent accidental mutation.Mark arrays
as const(or useReadonlyArray) to avoid runtime mutation and to preserve literal types.Apply this diff (example for all arrays):
-export const allTools = [ +export const allTools = [ /* ... */ -]; +] as const; -export const readOnlyRootTools = [ +export const readOnlyRootTools = [ /* ... */ -] +] as const; -const editOnlyRootTools = [ +const editOnlyRootTools = [ /* ... */ -] +] as const; -export const rootTools = [...readOnlyRootTools, ...editOnlyRootTools]; +export const rootTools = [...readOnlyRootTools, ...editOnlyRootTools] as const; -export const userTools = [ +export const userTools = [ ListBranchesTool, ListFilesTool, -] +] as const;Also applies to: 28-42, 43-52, 54-59
packages/ai/src/chat/providers.ts (1)
53-64: Optional: mirror API key check for Anthropic for early failure.For parity with OpenRouter, consider validating
ANTHROPIC_API_KEYand throwing a clear error.packages/ai/src/agents/tools/user.ts (1)
16-18: Handle empty tool labels gracefullyWhen the tool hasn’t received an input yet, this returns
Doggo: undefined, which leaks straight into the UI. Let’s fall back to a clean label until a message is actually available.- getLabel(input?: { message: string }): string { - return `Doggo: ${input?.message}`; + getLabel(input?: { message: string }): string { + const message = input?.message?.trim(); + return message ? `Doggo: ${message}` : 'Doggo'; }packages/ai/src/agents/onlook-chat.ts (1)
21-23: Clone initial messages to avoid shared mutationsRight now we stash the caller’s
initialMessagesarray by reference. If the caller keeps that array (common when rehydrating from previously cached chat state) and mutates it later, we silently mutate our internal state without firing any callbacks. Cloning up front keeps the state private and consistent with the deep-clone semantics you already rely on inreplaceMessage.constructor(initialMessages: UI_MESSAGE[] = []) { - this.#messages = initialMessages; + this.#messages = initialMessages.map(message => + structuredClone(message), + ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
apps/web/client/src/app/api/chat/helpers/stream.ts(2 hunks)apps/web/client/src/app/api/chat/route.ts(4 hunks)apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(5 hunks)apps/web/client/src/components/tools/tools.ts(2 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(4 hunks)apps/web/client/src/server/api/routers/chat/suggestion.ts(1 hunks)apps/web/client/src/server/api/routers/project/project.ts(1 hunks)packages/ai/src/agents/classes/index.ts(1 hunks)packages/ai/src/agents/classes/root.ts(2 hunks)packages/ai/src/agents/classes/user.ts(1 hunks)packages/ai/src/agents/index.ts(1 hunks)packages/ai/src/agents/models/base.ts(2 hunks)packages/ai/src/agents/onlook-chat.ts(1 hunks)packages/ai/src/agents/streamer.ts(2 hunks)packages/ai/src/agents/tool-lookup.ts(1 hunks)packages/ai/src/agents/tools/base.ts(1 hunks)packages/ai/src/agents/tools/index.ts(1 hunks)packages/ai/src/agents/tools/user.ts(1 hunks)packages/ai/src/chat/providers.ts(2 hunks)packages/ai/src/tools/index.ts(1 hunks)packages/ai/src/tools/models/client.ts(1 hunks)packages/ai/src/tools/toolset-utils.ts(1 hunks)packages/ai/src/tools/toolset.ts(1 hunks)packages/db/src/defaults/conversation.ts(2 hunks)packages/db/src/mappers/chat/conversation.ts(2 hunks)packages/db/src/schema/chat/conversation.ts(1 hunks)packages/models/src/chat/conversation/index.ts(1 hunks)packages/models/src/chat/message/message.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/agents/tools/index.tspackages/models/src/chat/message/message.tspackages/ai/src/agents/classes/index.tspackages/ai/src/tools/toolset-utils.tsapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/agents/streamer.tspackages/db/src/mappers/chat/conversation.tspackages/ai/src/tools/index.tspackages/ai/src/agents/tool-lookup.tsapps/web/client/src/server/api/routers/chat/conversation.tspackages/ai/src/tools/models/client.tsapps/web/client/src/components/tools/tools.tspackages/ai/src/agents/models/base.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxpackages/ai/src/agents/tools/base.tspackages/models/src/chat/conversation/index.tspackages/ai/src/agents/index.tspackages/db/src/schema/chat/conversation.tspackages/ai/src/agents/classes/user.tspackages/ai/src/agents/classes/root.tspackages/db/src/defaults/conversation.tspackages/ai/src/agents/onlook-chat.tspackages/ai/src/tools/toolset.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/api/chat/route.tspackages/ai/src/agents/tools/user.tspackages/ai/src/chat/providers.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/agents/tools/index.tspackages/models/src/chat/message/message.tspackages/ai/src/agents/classes/index.tspackages/ai/src/tools/toolset-utils.tsapps/web/client/src/app/api/chat/helpers/stream.tspackages/ai/src/agents/streamer.tspackages/db/src/mappers/chat/conversation.tspackages/ai/src/tools/index.tspackages/ai/src/agents/tool-lookup.tsapps/web/client/src/server/api/routers/chat/conversation.tspackages/ai/src/tools/models/client.tsapps/web/client/src/components/tools/tools.tspackages/ai/src/agents/models/base.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxpackages/ai/src/agents/tools/base.tspackages/models/src/chat/conversation/index.tspackages/ai/src/agents/index.tspackages/db/src/schema/chat/conversation.tspackages/ai/src/agents/classes/user.tspackages/ai/src/agents/classes/root.tspackages/db/src/defaults/conversation.tspackages/ai/src/agents/onlook-chat.tspackages/ai/src/tools/toolset.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/api/chat/route.tspackages/ai/src/agents/tools/user.tspackages/ai/src/chat/providers.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/components/tools/tools.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/project/project.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/api/chat/helpers/stream.tsapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/server/api/routers/chat/suggestion.tsapps/web/client/src/server/api/routers/project/project.ts
apps/web/client/src/app/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/app/**/*.tsx: Default to Server Components; add 'use client' when using events, state/effects, browser APIs, or client‑only libraries
Do not use process.env in client code; import env from @/env insteadAvoid hardcoded user-facing text; use next-intl messages/hooks
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
apps/web/client/src/**/*.tsx
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable references across renders
Keep the active MobX store in a useRef and perform async cleanup with setTimeout(() => storeRef.current?.clear(), 0) to avoid route-change races
Avoid useMemo for creating MobX store instances
Avoid putting the MobX store instance in effect dependency arrays if it causes loops; split concerns by domain
apps/web/client/src/**/*.tsx: Create MobX store instances with useState(() => new Store()) for stable identities across renders
Keep the active MobX store in a useRef and clean up asynchronously with setTimeout(() => storeRef.current?.clear(), 0)
Do not use useMemo to create MobX stores
Avoid placing MobX store instances in effect dependency arrays if it causes loops; split concerns instead
observer components must be client components; place a single client boundary at the feature entry; child observers need not repeat 'use client'
Files:
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
🧠 Learnings (2)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from apps/web/client/src/server/api/root.ts
Applied to files:
packages/ai/src/agents/classes/index.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/server/api/root.ts : Export all tRPC routers from src/server/api/root.ts
Applied to files:
packages/ai/src/agents/classes/index.ts
🧬 Code graph analysis (17)
apps/web/client/src/app/api/chat/helpers/stream.ts (1)
packages/ai/src/chat/providers.ts (1)
initModel(14-51)
packages/ai/src/agents/streamer.ts (1)
packages/ai/src/stream/index.ts (1)
convertToStreamMessages(5-23)
packages/db/src/mappers/chat/conversation.ts (1)
packages/models/src/chat/conversation/index.ts (1)
ChatConversation(8-16)
packages/ai/src/agents/tool-lookup.ts (1)
packages/ai/src/agents/tools/user.ts (1)
UserAgentTool(5-19)
apps/web/client/src/server/api/routers/chat/conversation.ts (2)
packages/db/src/schema/chat/conversation.ts (1)
conversations(13-23)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
packages/ai/src/tools/models/client.ts (2)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)
apps/web/client/src/components/tools/tools.ts (3)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/toolset.ts (1)
getAvailableTools(8-17)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)
packages/ai/src/agents/models/base.ts (2)
packages/models/src/llm/index.ts (1)
ModelConfig(34-39)packages/ai/src/tools/toolset-utils.ts (1)
createToolSet(4-9)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx (2)
packages/db/src/schema/chat/conversation.ts (1)
agentType(11-11)apps/web/client/src/components/tools/tools.ts (1)
handleToolCall(8-46)
packages/ai/src/agents/tools/base.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)packages/ai/src/agents/onlook-chat.ts (7)
OnlookChat(114-136)messages(43-45)messages(47-50)status(25-27)status(29-32)error(34-36)error(38-41)
packages/ai/src/agents/classes/user.ts (3)
packages/models/src/llm/index.ts (1)
ModelConfig(34-39)packages/ai/src/chat/providers.ts (1)
initModel(14-51)packages/ai/src/agents/tool-lookup.ts (1)
userTools(56-59)
packages/ai/src/agents/classes/root.ts (3)
packages/ai/src/agents/tool-lookup.ts (2)
readOnlyRootTools(28-42)rootTools(54-54)packages/models/src/llm/index.ts (1)
ModelConfig(34-39)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
packages/ai/src/tools/toolset.ts (2)
packages/ai/src/agents/tool-lookup.ts (3)
rootTools(54-54)userTools(56-59)allTools(4-25)packages/ai/src/tools/toolset-utils.ts (1)
createToolSet(4-9)
apps/web/client/src/server/api/routers/chat/suggestion.ts (1)
packages/ai/src/chat/providers.ts (1)
initModel(14-51)
apps/web/client/src/server/api/routers/project/project.ts (1)
packages/ai/src/chat/providers.ts (1)
initModel(14-51)
apps/web/client/src/app/api/chat/route.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
agentType(11-11)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)packages/ai/src/agents/classes/root.ts (1)
RootAgent(10-58)packages/ai/src/agents/classes/user.ts (1)
UserAgent(6-17)
packages/ai/src/chat/providers.ts (1)
packages/models/src/llm/index.ts (3)
InitialModelPayload(27-32)ModelConfig(34-39)MODEL_MAX_TOKENS(41-49)
🪛 Biome (2.1.2)
packages/ai/src/agents/tools/base.ts
[error] 71-151: Promise executor functions should not be async.
(lint/suspicious/noAsyncPromiseExecutor)
🔇 Additional comments (6)
packages/ai/src/tools/index.ts (1)
4-4: LGTM — re-export looks good.packages/ai/src/agents/tools/index.ts (1)
1-1: LGTM — exposing user tools via barrel is fine.packages/ai/src/chat/providers.ts (1)
14-51: LGTM — sync initModel and provider getters are consistent.Signatures and return types align with downstream usage; OpenRouter key check is good.
packages/ai/src/agents/index.ts (1)
3-3: LGTM — exporting tools from agents barrel is fine.packages/ai/src/agents/classes/index.ts (1)
1-2: LGTM — public UserAgent export added.packages/ai/src/agents/streamer.ts (1)
19-21: Confirmagent.streamTextreturn type
Ensurethis.agent.streamText(...)returns a synchronous result object exposing.toUIMessageStreamResponse, not a Promise.
…is required for ROOT agents and forbidden for others
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (6)
apps/web/client/src/app/api/chat/route.ts (2)
102-106: Move usage increment logic after agent instantiation for consistency.The current code increments usage before creating the agent, but the agent creation could fail (e.g., if
chatTypeis undefined despite validation). Consider moving the usage increment after successful agent creation to maintain logical consistency.Apply this diff to move usage increment after agent creation:
let agent: BaseAgent; if (agentType === AgentType.ROOT) { - if (chatType === ChatType.EDIT) { - usageRecord = await incrementUsage(req, traceId); - } - agent = new RootAgent(chatType!); + + if (chatType === ChatType.EDIT) { + usageRecord = await incrementUsage(req, traceId); + } } else if (agentType === AgentType.USER) {
123-124: Redundant nullish coalescing due to schema defaults.The
agentType ?? AgentType.ROOTis redundant since the schema already providesAgentType.ROOTas the default value. ThechatType ?? "null"is appropriate for telemetry purposes whenchatTypeis undefined for non-ROOT agents.Apply this diff to remove the redundant nullish coalescing:
- agentType: agentType ?? AgentType.ROOT, + agentType: agentType,packages/ai/src/agents/tools/base.ts (4)
36-38: Prefer sending the user’s message, not a JSON blob, by defaultDefaulting to JSON.stringify(input) makes the sub-agent read JSON text. Use the message field; derived tools can override if needed.
- createMessage(input: TInput): string { - return JSON.stringify(input, null, 2); - } + createMessage(input: TInput): string { + return input.message; + }
95-109: Remove any casts; narrow with type guardsMultiple (part as any) casts bypass type safety. Use in-guards or type predicates for state and tool parts.
Example (no behavior change):
- Define once near the top:
type MaybeState = { state?: string }; const hasState = (p: unknown): p is MaybeState => !!p && typeof p === 'object' && 'state' in (p as object);
- Then:
const toolCallParts = message.parts.filter(p => String(p.type).startsWith('tool-')); const hasStreamingText = message.parts.some( p => p.type === 'text' && hasState(p) && p.state !== 'done', ); const hasIncompleteToolCalls = message.role === 'assistant' && toolCallParts.some(p => p.type === 'tool-call' || (hasState(p) && p.state !== 'output-available' && p.state !== 'output-error'));As per coding guidelines
29-31: Instance state makes concurrency unsafeconversationId and messages are mutable on the tool instance. Reusing a tool concurrently can interleave messages. Document single-flight usage or isolate state per call (e.g., clone messages snapshot for the chat session, or create a new tool instance per run).
62-64: Hook up context from editorEngine (TODO)You noted this TODO. I can wire in editor-derived context (e.g., current file, selection, git branch) into metadata.context.
Want me to draft a minimal context collector and integrate it here?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/client/src/app/api/chat/route.ts(4 hunks)packages/ai/src/agents/streamer.ts(2 hunks)packages/ai/src/agents/tool-lookup.ts(1 hunks)packages/ai/src/agents/tools/base.ts(1 hunks)packages/ai/src/tools/toolset.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/ai/src/agents/tool-lookup.ts
- packages/ai/src/agents/streamer.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/api/chat/route.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
apps/web/client/src/app/api/chat/route.tspackages/ai/src/tools/toolset.tspackages/ai/src/agents/tools/base.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/api/chat/route.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
apps/web/client/src/app/api/chat/route.tspackages/ai/src/tools/toolset.tspackages/ai/src/agents/tools/base.ts
🧬 Code graph analysis (3)
apps/web/client/src/app/api/chat/route.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
agentType(11-11)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)packages/ai/src/agents/classes/root.ts (1)
RootAgent(10-58)packages/ai/src/agents/classes/user.ts (1)
UserAgent(6-17)
packages/ai/src/tools/toolset.ts (2)
packages/ai/src/agents/tool-lookup.ts (4)
readOnlyRootTools(28-42)rootTools(54-54)userTools(56-59)allTools(4-25)packages/ai/src/tools/toolset-utils.ts (1)
createToolSet(4-9)
packages/ai/src/agents/tools/base.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)packages/ai/src/agents/onlook-chat.ts (5)
OnlookChat(114-136)messages(43-45)messages(47-50)error(34-36)error(38-41)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
🔇 Additional comments (7)
packages/ai/src/tools/toolset.ts (1)
6-13: ASK sessions correctly gated to read-only toolsLine 9 now returns
readOnlyRootToolsforChatType.ASK, so ASK conversations stay on the safe, read-only toolset while other ROOT sessions keep edit tools. Thanks for restoring the guardrail.apps/web/client/src/app/api/chat/route.ts (3)
61-81: The schema validation properly addresses the previous review feedback.The superRefine implementation correctly ensures that
chatTypeis required whenagentTypeis ROOT and forbidden otherwise. This prevents the undefinedchatTypeissue that was flagged in the previous review.
106-106: Non-null assertion is safe due to schema validation.The
chatType!non-null assertion is justified here because the schema validation ensures thatchatTypeis defined whenagentTypeis ROOT. The superRefine validation prevents this path from being reached with undefinedchatType.
66-67: Use proper ZodIssueCode in Zod 4.In Zod 4, ZodIssueCode is no longer an enum and the proper way to specify custom error codes has changed. The string 'custom' should be used directly instead of ZodIssueCode.custom.
Apply this diff to use the correct error code format:
- code: 'custom', + code: z.ZodIssueCode.custom,Or alternatively, use the string directly:
code: 'custom',The current implementation is actually correct for Zod 4. No change needed.
packages/ai/src/agents/tools/base.ts (3)
83-86: Correct binding for addToolResultPassing onlookChat.addToolResult.bind(onlookChat) fixes the previous this-loss bug. Good catch.
88-90: Reduce noisy console logs or gate behind a debug flagReplace ad-hoc console.log/error with a configurable logger or guard with NODE_ENV/debug toggle to avoid leaking internal state in production.
Example:
const debug = process.env.NODE_ENV !== 'production' && process.env.ONLOOK_DEBUG === '1'; if (debug) console.log('OnlookChat - Finish:', options);Also applies to: 96-96, 113-116, 131-131, 134-134, 153-156
144-151: Ensure onFinish never fires before promise handlers are set
Handlers are bound before calling sendMessage(), but if OnlookChat’s constructor or AbstractChat ever invokes onFinish synchronously it would reference uninitialized promiseResolve/promiseReject and throw—verify the provider never triggers onFinish in the constructor.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ai/src/tools/toolset-utils.ts (1)
5-8: Prefer reduce generics over assertionWe can drop the
as ToolSetby lettingreducecarry the accumulator type, which tightens typing without the cast.-export function createToolSet(toolClasses: Array<{ toolName: string; getAITool: () => ToolSet[string] }>): ToolSet { - return toolClasses.reduce((acc, toolClass) => { +export function createToolSet(toolClasses: Array<{ toolName: string; getAITool: () => ToolSet[string] }>): ToolSet { + return toolClasses.reduce<ToolSet>((acc, toolClass) => { acc[toolClass.toolName] = toolClass.getAITool(); return acc; - }, {} as ToolSet); + }, {}); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx(6 hunks)packages/ai/src/tools/toolset-utils.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsx
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/tools/toolset-utils.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/tools/toolset-utils.ts
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/app/api/chat/route.ts(4 hunks)packages/ai/src/agents/tools/base.ts(1 hunks)packages/scripts/src/backend.ts(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/agents/tools/base.tspackages/scripts/src/backend.tsapps/web/client/src/app/api/chat/route.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/agents/tools/base.tspackages/scripts/src/backend.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/api/chat/route.ts
🧠 Learnings (3)
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables in src/env.ts via t3-oss/env-nextjs
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables via t3-oss/env-nextjs in apps/web/client/src/env.ts
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/env.ts : Expose browser environment variables with NEXT_PUBLIC_* and declare them in the client schema
Applied to files:
packages/scripts/src/backend.ts
🧬 Code graph analysis (3)
packages/ai/src/agents/tools/base.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)packages/ai/src/agents/onlook-chat.ts (5)
OnlookChat(114-136)messages(43-45)messages(47-50)error(34-36)error(38-41)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
packages/scripts/src/backend.ts (3)
.vscode/.debug.script.mjs (1)
__dirname(8-8)packages/scripts/src/index.ts (1)
rootDir(12-12)packages/scripts/src/helpers.ts (1)
writeEnvFile(116-176)
apps/web/client/src/app/api/chat/route.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
agentType(11-11)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)packages/ai/src/agents/classes/root.ts (1)
RootAgent(10-58)packages/ai/src/agents/classes/user.ts (1)
UserAgent(6-17)
🔇 Additional comments (4)
apps/web/client/src/app/api/chat/route.ts (1)
55-67: Restore stricter chatType validation for ROOT agentsWith the current schema, a ROOT request can omit
chatType, slip through validation, and we end up callingnew RootAgent(chatType!)withundefined. That silently downgrades to the EDIT defaults and, more importantly, bypasses the usage increment because the later guardchatType === ChatType.EDITnever fires. This is the same issue called out in the earlier review and still needs to be fixed. Please requirechatTypewheneveragentTypeis ROOT and continue forbidding it for non-root agents.-}).refine((data) => { - // Only allow chatType if agentType is ROOT - if (data.chatType !== undefined && data.agentType !== AgentType.ROOT) { - return false; - } - return true; -}, { message: "chatType is only allowed if agentType is root" }); +}).superRefine((data, ctx) => { + if (data.agentType === AgentType.ROOT) { + if (data.chatType === undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['chatType'], + message: 'chatType is required when agentType is root', + }); + } + return; + } + if (data.chatType !== undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['chatType'], + message: 'chatType is only allowed if agentType is root', + }); + } +});packages/ai/src/agents/tools/base.ts (3)
188-191: AssertconversationIdis non-null instead of hiding it behind a castAt Line 189 we still widen
conversationIdtostring | nulland then erase the mismatch withas TOutput. Because_conversationIdis guaranteed earlier, assert it explicitly (or throw) so TypeScript can enforce the invariant and we keep the cast focused on the generic.- return { - conversationId: this.conversationId, + return { + conversationId: this.conversationId!, response, } as TOutput;
122-134: Resolve completion when the final part is a tool resultLine 122 currently insists the final part be text with
state === 'done', so any completion that ends with a tool output (tool-result,tool-error, etc.) never flipslastPartCompleteto true. In that casepromiseResolveis never called and the awaiting sub-agent hangs indefinitely. Please treat completed tool outputs as final (or drop the check entirely) so we resolve onceisIncompleteis false.- const lastPart = message.parts[message.parts.length - 1]; - const lastPartComplete = lastPart && - (lastPart.type === 'text' && (lastPart as any).state === 'done'); + const lastPart = message.parts[message.parts.length - 1]; + const lastPartState = (lastPart as any)?.state; + const lastPartComplete = + (lastPart?.type === 'text' && lastPartState === 'done') || + (typeof lastPart?.type === 'string' && + lastPart.type.startsWith('tool-') && + (lastPartState === 'output-available' || lastPartState === 'output-error'));
176-178: Fix metadata error handling to avoid returningundefinedLine 176 treats
metadata.error !== nullas the guard, so when the value isundefinedwe still try toJSON.stringifyit; the result isundefined, which then propagates toresponseif no later branch overwrites it. When the message has no parts (or parsing a tool-only response) we end up returning{ response: undefined }, violating the declared contract. Guard with a nullish check and preserve the empty-string fallback.- if (agentResponse.metadata?.error !== null) { - response = JSON.stringify(agentResponse.metadata!.error, null, 2); - } + const errorMetadata = agentResponse.metadata?.error; + if (errorMetadata != null) { + response = JSON.stringify(errorMetadata, null, 2) ?? ''; + }
samschooler
force-pushed
the
subagents
branch
from
5aa588f to
970953f
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/ai/src/agents/streamer.ts (1)
16-24: Reintroduce awaiting ofagent.streamTextresult
BaseAgent.streamTextstill resolves aPromisefromai.streamText. Droppingasync/awaitmeansresultis aPromise, soresult.toUIMessageStreamResponse(...)will throw (Promisehas no such method). This is a hard runtime break for every stream. Please restore theasyncfunction andawaitthe call.- streamText(messages: ChatMessage[], { streamTextConfig, toUIMessageStreamResponseConfig }: { streamTextConfig: Omit<Partial<Parameters<typeof streamText>[0]>, 'model' | 'headers' | 'tools' | 'stopWhen' | 'messages' | 'prompt'>, toUIMessageStreamResponseConfig: UIMessageStreamOptions<ChatMessage> }) { + async streamText(messages: ChatMessage[], { streamTextConfig, toUIMessageStreamResponseConfig }: { streamTextConfig: Omit<Partial<Parameters<typeof streamText>[0]>, 'model' | 'headers' | 'tools' | 'stopWhen' | 'messages' | 'prompt'>, toUIMessageStreamResponseConfig: UIMessageStreamOptions<ChatMessage> }) { const conversationId = this.conversationId; - const result = this.agent.streamText(convertToStreamMessages(messages), streamTextConfig); + const result = await this.agent.streamText(convertToStreamMessages(messages), streamTextConfig);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (6)
apps/web/client/src/app/api/chat/route.ts(4 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(2 hunks)packages/ai/src/agents/streamer.ts(2 hunks)packages/ai/src/agents/tools/base.ts(1 hunks)packages/db/src/schema/chat/conversation.ts(2 hunks)packages/scripts/src/backend.ts(12 hunks)
🧰 Additional context used
📓 Path-based instructions (5)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/db/src/schema/chat/conversation.tspackages/ai/src/agents/tools/base.tsapps/web/client/src/server/api/routers/chat/conversation.tspackages/scripts/src/backend.tspackages/ai/src/agents/streamer.tsapps/web/client/src/app/api/chat/route.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/db/src/schema/chat/conversation.tspackages/ai/src/agents/tools/base.tsapps/web/client/src/server/api/routers/chat/conversation.tspackages/scripts/src/backend.tspackages/ai/src/agents/streamer.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/conversation.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/server/api/routers/chat/conversation.tsapps/web/client/src/app/api/chat/route.ts
apps/web/client/src/app/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Default to Server Components; add 'use client' only when using events, state/effects, browser APIs, or client-only libs
Files:
apps/web/client/src/app/api/chat/route.ts
🧠 Learnings (5)
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/env.ts : Expose browser environment variables with NEXT_PUBLIC_* and declare them in the client schema
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables in src/env.ts via t3-oss/env-nextjs
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/src/env.ts : Define and validate environment variables via t3-oss/env-nextjs in apps/web/client/src/env.ts
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-14T01:44:21.209Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: AGENTS.md:0-0
Timestamp: 2025-09-14T01:44:21.209Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in Next.js config to enforce env validation at build time
Applied to files:
packages/scripts/src/backend.ts
📚 Learning: 2025-09-16T19:22:52.461Z
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-16T19:22:52.461Z
Learning: Applies to apps/web/client/next.config.ts : Import ./src/env in next.config.ts to enforce env validation at build time
Applied to files:
packages/scripts/src/backend.ts
🧬 Code graph analysis (5)
packages/ai/src/agents/tools/base.ts (5)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)packages/ai/src/agents/onlook-chat.ts (5)
OnlookChat(114-136)messages(43-45)messages(47-50)error(34-36)error(38-41)apps/web/client/src/trpc/react.tsx (1)
api(23-23)
apps/web/client/src/server/api/routers/chat/conversation.ts (2)
packages/db/src/schema/chat/conversation.ts (1)
conversations(13-23)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
packages/scripts/src/backend.ts (2)
packages/scripts/src/helpers.ts (1)
writeEnvFile(116-176)packages/scripts/src/index.ts (1)
rootDir(12-12)
packages/ai/src/agents/streamer.ts (3)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)packages/ai/src/agents/models/base.ts (1)
streamText(28-43)packages/ai/src/stream/index.ts (1)
convertToStreamMessages(5-23)
apps/web/client/src/app/api/chat/route.ts (4)
packages/db/src/schema/chat/conversation.ts (1)
agentType(11-11)apps/web/client/src/app/api/chat/helpers/usage.ts (1)
incrementUsage(43-65)packages/ai/src/agents/classes/root.ts (1)
RootAgent(10-58)packages/ai/src/agents/classes/user.ts (1)
UserAgent(6-17)
🔇 Additional comments (5)
packages/scripts/src/backend.ts (1)
16-45: Still returning the wrong repo root when.gitis missingWe’re still hanging onto the first
package.jsonwe encounter, which makesrootDirpoint topackages/scripts(or any nested workspace) if the checkout lacks.git—exactly the failure called out in the earlier review. Keep walking upward and remember the lastpackage.jsonso we fall back to the real repository root in ZIP/CI environments.- let firstPackageJsonDir: string | null = null; + let lastPackageJsonDir: string | null = null; … - if (fs.existsSync(packageJsonPath) && !firstPackageJsonDir) { - firstPackageJsonDir = currentDir; + if (fs.existsSync(packageJsonPath)) { + lastPackageJsonDir = currentDir; } … - if (firstPackageJsonDir) { - return firstPackageJsonDir; + if (lastPackageJsonDir) { + return lastPackageJsonDir; }packages/ai/src/agents/tools/base.ts (2)
117-135: Fix unresolved completion hang for tool outputsWe still only resolve when the final part is text with state
done. When the assistant finishes with a completed tool payload (e.g.,tool-resultinoutput-available/output-error), this branch never runs and the promise never settles, stalling sub-agent flows. Please accept completed tool outputs as final before resolving.Apply this diff:
- const lastPart = message.parts[message.parts.length - 1]; - const lastPartComplete = lastPart && - (lastPart.type === 'text' && (lastPart as any).state === 'done'); - - console.log("OnlookChat - Last part:", { type: lastPart?.type, state: (lastPart as any)?.state }); - console.log("OnlookChat - Last part complete:", lastPartComplete); - - if (!isIncomplete && lastPartComplete) { + const lastPart = message.parts[message.parts.length - 1]; + const lastPartState = (lastPart as any)?.state; + const lastPartComplete = + (lastPart?.type === 'text' && lastPartState === 'done') || + (lastPart?.type?.startsWith('tool-') && + (lastPartState === 'output-available' || lastPartState === 'output-error')); + + console.log("OnlookChat - Last part:", { type: lastPart?.type, state: lastPartState }); + console.log("OnlookChat - Last part complete:", lastPartComplete); + + if (!isIncomplete && lastPartComplete) {
176-178: Guard metadata.error before serializing
metadata.erroris oftenundefined, so the current!== nullcheck fires,JSON.stringify(undefined)returnsundefined, andresponsestops being a string if the message has no text parts. Please null-check the error payload first.Apply this diff:
- if (agentResponse.metadata?.error !== null) { - response = JSON.stringify(agentResponse.metadata!.error, null, 2); - } + const metadataError = agentResponse.metadata?.error; + if (metadataError != null) { + response = JSON.stringify(metadataError, null, 2) ?? ''; + }packages/db/src/schema/chat/conversation.ts (1)
11-11: Fix pgEnum initialization to use string values
pgEnumexpects an iterable/tuple of string literals. Passing the enum object causes Drizzle to blow up when it tries to iterate (e.g.,.map) over the enum definition at runtime. Please supply the string values, e.g.Object.values(AgentType) as [AgentType, ...AgentType[]], before callingpgEnum.-export const agentType = pgEnum("agent_type", AgentType); +const agentTypeValues = Object.values(AgentType) as [AgentType, ...AgentType[]]; +export const agentType = pgEnum("agent_type", agentTypeValues);apps/web/client/src/app/api/chat/route.ts (1)
55-67: RequirechatTypewheneveragentTypeis ROOTRight now the schema lets a ROOT request omit
chatType, sochatType!later becomesundefined. That silently instantiatesRootAgentin the EDIT branch, skips the usage increment, and ships"null"telemetry. Please hard-requirechatTypefor ROOT (while still forbidding it for other agent types) so we never hydrate a root agent with an undefined mode.-}).refine((data) => { - // Only allow chatType if agentType is ROOT - if (data.chatType !== undefined && data.agentType !== AgentType.ROOT) { - return false; - } - return true; -}, { message: "chatType is only allowed if agentType is root" }); +}).superRefine((data, ctx) => { + if (data.agentType === AgentType.ROOT && data.chatType === undefined) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['chatType'], + message: 'chatType is required when agentType is root', + }); + } + if (data.chatType !== undefined && data.agentType !== AgentType.ROOT) { + ctx.addIssue({ + code: z.ZodIssueCode.custom, + path: ['chatType'], + message: 'chatType is only allowed if agentType is root', + }); + } +});
…ds and streamlining message handling
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web/client/src/components/store/editor/chat/conversation.ts(1 hunks)apps/web/client/src/server/api/routers/chat/conversation.ts(3 hunks)packages/ai/src/agents/tools/base.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Do not use the any type unless necessary
Files:
packages/ai/src/agents/tools/base.tsapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/server/api/routers/chat/conversation.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/agents/tools/base.tsapps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/server/api/routers/chat/conversation.ts
apps/web/client/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/**/*.{ts,tsx}: Use path aliases @/* and ~/* for imports that map to apps/web/client/src/*
Avoid hardcoded user-facing text; use next-intl messages/hooks insteadUse path aliases @/* and ~/* for imports mapping to src/*
Files:
apps/web/client/src/components/store/editor/chat/conversation.tsapps/web/client/src/server/api/routers/chat/conversation.ts
apps/web/client/src/server/api/routers/**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under apps/web/client/src/server/api/routers/**
Use publicProcedure/protectedProcedure from apps/web/client/src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization in tRPC procedures
apps/web/client/src/server/api/routers/**/*.ts: Place tRPC routers under src/server/api/routers/**
Use publicProcedure/protectedProcedure from src/server/api/trpc.ts and validate inputs with Zod
Return plain objects/arrays; rely on SuperJSON for serialization
Files:
apps/web/client/src/server/api/routers/chat/conversation.ts
🧬 Code graph analysis (3)
packages/ai/src/agents/tools/base.ts (4)
packages/models/src/chat/message/message.ts (1)
ChatMessage(19-19)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(34-140)packages/ai/src/tools/models/client.ts (1)
OnToolCallHandler(6-6)packages/ai/src/agents/onlook-chat.ts (5)
OnlookChat(114-136)messages(43-45)messages(47-50)error(34-36)error(38-41)
apps/web/client/src/components/store/editor/chat/conversation.ts (1)
packages/models/src/chat/conversation/index.ts (1)
ChatConversation(8-16)
apps/web/client/src/server/api/routers/chat/conversation.ts (2)
packages/db/src/schema/chat/conversation.ts (1)
conversations(13-23)packages/ai/src/chat/providers.ts (1)
initModel(14-51)
🔇 Additional comments (2)
packages/ai/src/agents/tools/base.ts (1)
121-134: Unblock completion when the final chunk is a tool outputWe still only resolve after a text part ends with state
"done". When the assistant finishes on a completed tool output (tool-resultwithoutput-available/output-error), the promise never resolves and sub-agent calls hang. Treat completed tool outputs as terminal (or rely solely onisIncomplete) before resolving. See earlier feedback—the bug remains.Apply something like:
- const lastPart = message.parts[message.parts.length - 1]; - const lastPartComplete = lastPart && - (lastPart.type === 'text' && (lastPart as any).state === 'done'); + const lastPart = message.parts[message.parts.length - 1]; + const lastPartState = (lastPart as any)?.state; + const lastPartComplete = + lastPart && + ( + (lastPart.type === 'text' && lastPartState === 'done') || + (lastPart.type.startsWith('tool-') && + (lastPartState === 'output-available' || lastPartState === 'output-error')) + );apps/web/client/src/server/api/routers/chat/conversation.ts (1)
54-58: Filter undefined fields before update
inputis a partial payload, so spreading it directly into.set(...)can persistNULLs for any keys that happen to beundefined. Please reuse the filtered map from the upsert fix when building the update payload, keepingupdatedAtforced.- const [conversation] = await ctx.db.update(conversations) - .set({ - ...input, - updatedAt: new Date(), - }) + const updateValues = Object.fromEntries( + Object.entries(input).filter(([key, value]) => key !== 'id' && value !== undefined), + ); + const [conversation] = await ctx.db.update(conversations) + .set({ + ...updateValues, + updatedAt: new Date(), + })
2 participants
Description
refactor: initModel to sync
refactor: toolset-lookup/registry
refactor: useChat to user agenttype.root by default
refactor: tools to use tools for agenttype
feat: onlook-chat: implementation of AbstractChat from ai sdk
feat: SubAgent tool arch allowing subagents to call tools on the client within their own threads
feat: Example SubAgent -- allows to list files
Type of Change
Important
Introduces SubAgent architecture for client-side tool calls, refactors model initialization, and updates chat and tool handling across the codebase.
SubAgentarchitecture allowing subagents to call tools on the client within their own threads.UserAgentandRootAgentclasses inpackages/ai/src/agents/classes.OnlookChatinpackages/ai/src/agents/onlook-chat.tsfor chat state management.initModelto synchronous inpackages/ai/src/chat/providers.tsand other files.toolsandagentsdirectories.useChathook inapps/web/client/src/app/project/[id]/_hooks/use-chat/index.tsxto useagentType.POSTfunction inapps/web/client/src/app/api/chat/route.tsto handle differentagentType.conversationRouterandsuggestionsRouterinapps/web/client/src/server/api/routers/chatto support new agent types.agentTypetoconversationsschema inpackages/db/src/schema/chat/conversation.ts.agentTypeinpackages/db/src/mappers/chat/conversation.tsandpackages/db/src/defaults/conversation.ts.This description was created by
for fdccec9. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
New Features
Improvements
Bug Fixes