feat: added typecheck tool #2783
Conversation
|
@Paribesh01 is attempting to deploy a commit to the Onlook Team on Vercel. A member of the Team first needs to authorize it. |
|
Caution Review failedThe pull request is closed. WalkthroughAdds an end-to-end "typecheck" tool: CLI tool definition and toolset registration, client handler to run a sandboxed Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Agent
participant AI_Toolset as AI Toolset
participant Client as Web Client
participant Sandbox as EditorEngine.sandbox
Agent->>AI_Toolset: request "typecheck"
AI_Toolset->>Agent: emit tool invocation (TYPECHECK_TOOL_NAME)
Agent->>Client: invoke client handler (TYPECHECK_TOOL_NAME)
Client->>Sandbox: runCommand("bunx tsc --noEmit")
Sandbox-->>Client: { exitCode, output, error }
Client-->>Agent: { success: bool, error?: string }
Note right of Client: UI renders result — stdout on success\nor sanitized stderr on failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Assessment against linked issues
Out-of-scope changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 💡 Knowledge Base configuration:
You can enable these sources in your CodeRabbit configuration. 📒 Files selected for processing (1)
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
|
hey @Kitenite i have added a typecheck tool, can you plz review this pr |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/web/client/src/components/tools/tools.ts (1)
172-179: Rename unused handler parameter and optionally add a preflight check for the typecheck scriptFile: apps/web/client/src/components/tools/tools.ts (lines 172–179)
- handler: async (args: z.infer<typeof TYPECHECK_TOOL_PARAMETERS>, editorEngine: EditorEngine) => { + handler: async (_args: z.infer<typeof TYPECHECK_TOOL_PARAMETERS>, editorEngine: EditorEngine) => {
- Rename
argsto_argsin the handler signature to satisfy no-unused-parameters.- Optional: before running
bun run typecheck, verify the workspace’spackage.jsondefines a"typecheck"script (most packages lack it: apps/backend, apps/web, tooling/typescript) and throw a clear error if missing.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/client/src/components/tools/tools.ts(2 hunks)packages/ai/src/tools/cli.ts(1 hunks)packages/ai/src/tools/toolset.ts(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T03:33:58.606Z
Learning: Run type checking with `bun run typecheck`
🧬 Code graph analysis (2)
apps/web/client/src/components/tools/tools.ts (3)
packages/ai/src/tools/cli.ts (2)
TYPECHECK_TOOL_NAME(104-104)TYPECHECK_TOOL_PARAMETERS(105-105)apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(31-111)apps/web/client/src/components/tools/handlers/cli.ts (1)
handleTerminalCommandTool(13-22)
packages/ai/src/tools/toolset.ts (1)
packages/ai/src/tools/cli.ts (2)
TYPECHECK_TOOL_NAME(104-104)typecheckTool(106-110)
🔇 Additional comments (4)
packages/ai/src/tools/cli.ts (1)
104-110: Typecheck tool definition — LGTMWell-scoped addition with empty input schema and a clear description. No issues.
packages/ai/src/tools/toolset.ts (2)
13-15: Wiring imports — LGTMImports for TYPECHECK tool are correct and consistent with local conventions.
65-65: Build-only registration — LGTMAdded only to BUILD_TOOL_SET (not ASK), which matches the intended usage.
apps/web/client/src/components/tools/tools.ts (1)
34-35: Client-side tool import — LGTMImports align with the AI package exports. No concerns.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/web/client/src/components/tools/handlers/cli.ts (2)
151-156: Type the args with the zod schema and mark as intentionally unusedUse the exported TYPECHECK_TOOL_PARAMETERS for consistency with other handlers and prefix the unused param to satisfy linters.
-import type { EditorEngine } from '@/components/store/editor/engine'; +import type { EditorEngine } from '@/components/store/editor/engine'; import { ALLOWED_BASH_EDIT_COMMANDS, ALLOWED_BASH_READ_COMMANDS, BASH_EDIT_TOOL_PARAMETERS, BASH_READ_TOOL_PARAMETERS, GLOB_TOOL_PARAMETERS, GREP_TOOL_PARAMETERS, - TERMINAL_COMMAND_TOOL_PARAMETERS + TERMINAL_COMMAND_TOOL_PARAMETERS, + TYPECHECK_TOOL_PARAMETERS } from '@onlook/ai'; @@ -export async function handleTypecheckTool( - args: Record<string, never>, +export async function handleTypecheckTool( + _args: z.infer<typeof TYPECHECK_TOOL_PARAMETERS>, editorEngine: EditorEngine, ): Promise<{ success: boolean; error?: string; }> {
158-171: Avoid “false green” UI by exposing stdout and keep command configurableReturn result.output on success (useful warnings), and surface both error and output on failure. Also, consider centralizing the command string to avoid drift with the UI.
- try { - // Run Next.js typecheck command - const result = await editorEngine.sandbox.session.runCommand('bunx tsc --noEmit'); + try { + const cmd = 'bunx tsc --noEmit'; + const result = await editorEngine.sandbox.session.runCommand(cmd); - if (result.success) { - return { - success: true - }; - } else { - return { - success: false, - error: result.error || result.output || 'Typecheck failed with unknown error' - }; - } + if (result.success) { + return { success: true /* optionally: , output: result.output */ }; + } + return { success: false, error: result.error || result.output || 'Typecheck failed with unknown error' };
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (6)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx(0 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx(3 hunks)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsx(3 hunks)apps/web/client/src/components/tools/handlers/cli.ts(1 hunks)apps/web/client/src/components/tools/tools.ts(3 hunks)packages/ai/src/tools/cli.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/index.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/client/src/components/tools/tools.ts
- packages/ai/src/tools/cli.ts
🧰 Additional context used
🧬 Code graph analysis (3)
apps/web/client/src/components/tools/handlers/cli.ts (1)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(32-115)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsx (2)
packages/ai/src/tools/cli.ts (1)
TYPECHECK_TOOL_NAME(104-104)packages/ui/src/components/icons/index.tsx (1)
Icons(137-3592)
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx (5)
packages/ai/src/tools/cli.ts (2)
TERMINAL_COMMAND_TOOL_NAME(4-4)TYPECHECK_TOOL_NAME(104-104)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsx (1)
ToolCallSimple(60-207)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/code-display/bash-code-display.tsx (1)
BashCodeDisplay(60-151)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/code-display/search-sources-display.tsx (1)
SearchSourcesDisplay(18-86)apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/code-display/collapsible-code-block.tsx (1)
CollapsibleCodeBlock(21-125)
🔇 Additional comments (3)
apps/web/client/src/components/tools/handlers/cli.ts (1)
160-161: Usetsc --noEmitdirectly for Next.js type checks
Next.js does not provide a built-innext typecheckcommand—its docs explicitly recommend runningtsc --noEmitfor CI/type checking. You may optionally define a"typecheck": "tsc --noEmit"script in package.json and invoke that, but retainingbunx tsc --noEmitis correct if no custom script exists.apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-simple.tsx (1)
27-32: LGTM: tool name import, icon mapping, and labelThe TYPECHECK tool import, icon mapping, and “Checking types” label look consistent with the rest of the component.
Also applies to: 57-58, 175-176
apps/web/client/src/app/project/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx (1)
19-20: strip-ansi dependency verified: Found in apps/web/client/package.json; no action required.
...ct/[id]/_components/right-panel/chat-tab/chat-messages/message-content/tool-call-display.tsx
Show resolved
Hide resolved
Kitenite
left a comment
Kitenite
left a comment
There was a problem hiding this comment.
@Paribesh01 i don't believe your original implementation works because bun run typecheck does not actually exist as a command outside of the app. I fixed it up and added display.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/ai/src/prompt/system.ts (1)
10-10: Reword the directive to scope typecheck tool calls
- In packages/ai/src/prompt/system.ts, replace the line:
- - ALWAYS run typecheck after making code changes to ensure type safety. Use the typecheck tool to verify your changes don't introduce type errors.with:
+ - After making changes that may affect types, run the typecheck tool (preferred over terminal) at least once before finishing an edit. If it fails, summarize and fix or ask for input; if it passes, note "✅ Typecheck passed!"
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (3)
apps/web/client/src/components/store/editor/chat/conversation.ts(0 hunks)packages/ai/src/prompt/system.ts(1 hunks)packages/ai/src/tools/cli.ts(1 hunks)
💤 Files with no reviewable changes (1)
- apps/web/client/src/components/store/editor/chat/conversation.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/ai/src/tools/cli.ts
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: CR
PR: onlook-dev/onlook#0
File: CLAUDE.md:0-0
Timestamp: 2025-08-31T03:33:58.606Z
Learning: Run type checking with `bun run typecheck`
2 participants
close #2759
Important
Adds a new typecheck tool to ensure type safety, integrating it into the toolset and handlers.
TYPECHECK_TOOL_NAMEandTYPECHECK_TOOL_PARAMETERSincli.ts.typecheckToolincli.tswith a description and empty input schema.typecheckTooltoBUILD_TOOL_SETintoolset.ts.TOOL_HANDLERSintools.tsto includeTYPECHECK_TOOL_NAMEwith a handler that runsbun run typecheck.This description was created by
for 2a0a474. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit