-
Notifications
You must be signed in to change notification settings - Fork 1.7k
Toolcall for creating multiple branches from a prompt #3044
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…nchesTool in exports
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
WalkthroughThe pull request extends the branch forking API with optional custom frame positioning via Changes
Sequence DiagramsequenceDiagram
participant User
participant EditorEngine
participant CreateMultipleBranchesTool
participant BranchAPI
participant SandboxManager
participant EditorUI
User->>EditorEngine: Invoke create_multiple_branches<br/>(count=3)
EditorEngine->>CreateMultipleBranchesTool: handle(args, editorEngine)
CreateMultipleBranchesTool->>BranchAPI: Get source branch frames<br/>for positioning
BranchAPI-->>CreateMultipleBranchesTool: Frames data
rect rgb(220, 240, 250)
note over CreateMultipleBranchesTool: Loop: Fork each branch<br/>with horizontal positioning
loop For each branch (1 to count)
CreateMultipleBranchesTool->>BranchAPI: fork(positionOverride: {x, y})
BranchAPI-->>CreateMultipleBranchesTool: New branch, sandbox, frames
CreateMultipleBranchesTool->>EditorUI: Register branch in<br/>editor's branch map
CreateMultipleBranchesTool->>SandboxManager: Initialize sandbox
end
end
rect rgb(240, 220, 250)
note over CreateMultipleBranchesTool: Poll & Track readiness
par Parallel polling
CreateMultipleBranchesTool->>SandboxManager: Poll branch 1 readiness
CreateMultipleBranchesTool->>SandboxManager: Poll branch 2 readiness
CreateMultipleBranchesTool->>SandboxManager: Poll branch 3 readiness
and
SandboxManager-->>CreateMultipleBranchesTool: Ready / Timeout
SandboxManager-->>CreateMultipleBranchesTool: Ready / Timeout
SandboxManager-->>CreateMultipleBranchesTool: Ready / Timeout
end
end
CreateMultipleBranchesTool-->>EditorEngine: Summary: "Created 3 branches<br/>(2 ready, 1 timed out)"
EditorEngine-->>User: Result & Status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
| }; | ||
|
|
||
| // Access the private branchMap to add the branch data | ||
| (editorEngine.branches as any).branchMap.set(result.branch.id, branchData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Directly mutating a private property via (editorEngine.branches as any).branchMap.set bypasses encapsulation. Consider exposing a dedicated method on editorEngine.branches to safely add branch data.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/web/client/src/server/api/routers/project/branch.ts(2 hunks)packages/ai/src/tools/classes/create-multiple-branches.ts(1 hunks)packages/ai/src/tools/classes/index.ts(1 hunks)packages/ai/src/tools/toolset.ts(2 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/tools/classes/index.tsapps/web/client/src/server/api/routers/project/branch.tspackages/ai/src/tools/toolset.tspackages/ai/src/tools/classes/create-multiple-branches.ts
{apps,packages}/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Avoid using the any type unless absolutely necessary
Files:
packages/ai/src/tools/classes/index.tsapps/web/client/src/server/api/routers/project/branch.tspackages/ai/src/tools/toolset.tspackages/ai/src/tools/classes/create-multiple-branches.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/project/branch.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/project/branch.ts
🧬 Code graph analysis (2)
packages/ai/src/tools/toolset.ts (2)
packages/ai/src/tools/classes/create-multiple-branches.ts (1)
CreateMultipleBranchesTool(18-195)packages/ai/src/tools/classes/index.ts (1)
CreateMultipleBranchesTool(4-4)
packages/ai/src/tools/classes/create-multiple-branches.ts (5)
apps/web/client/src/components/store/editor/engine.ts (1)
EditorEngine(33-135)packages/file-system/src/code-fs.ts (1)
CodeFileSystem(31-276)apps/web/client/src/components/store/editor/error/index.ts (1)
ErrorManager(5-75)apps/web/client/src/components/store/editor/sandbox/index.ts (1)
SandboxManager(20-224)apps/web/client/src/components/store/editor/history/index.ts (1)
HistoryManager(23-142)
| const originalActiveBranchId = editorEngine.branches.activeBranch.id; | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Handle absent active branch when preserving selection
If the caller supplies branchId, we can legitimately run with no active branch selected. In that case editorEngine.branches.activeBranch is undefined and this line throws before any forks happen. The final restoration block has the same assumption, so even if we got past the first access we would still blow up when trying to switch back. Please guard both sites so the tool works when no branch is currently active.
Apply this diff to harden the guard:
- const originalActiveBranchId = editorEngine.branches.activeBranch.id;
+ const originalActiveBranchId = editorEngine.branches.activeBranch?.id ?? null;
@@
- if (editorEngine.branches.activeBranch.id !== originalActiveBranchId) {
- await editorEngine.branches.switchToBranch(originalActiveBranchId);
- }
+ if (
+ originalActiveBranchId &&
+ editorEngine.branches.activeBranch?.id !== originalActiveBranchId
+ ) {
+ await editorEngine.branches.switchToBranch(originalActiveBranchId);
+ }Also applies to: 162-165
🤖 Prompt for AI Agents
In packages/ai/src/tools/classes/create-multiple-branches.ts around lines 77-78
(and similarly lines 162-165), the code assumes
editorEngine.branches.activeBranch is always defined; this throws when no active
branch exists. Fix by guarding accesses: store originalActiveBranchId using
optional chaining (e.g. const originalActiveBranchId =
editorEngine.branches.activeBranch?.id) or an explicit null check, and in the
final restoration block only attempt to switch back if originalActiveBranchId is
defined (skip restore when undefined). Ensure both the initial capture and the
restore use the same guard so the tool works when no branch is currently active.
| // Access the private branchMap to add the branch data | ||
| (editorEngine.branches as any).branchMap.set(result.branch.id, branchData); | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Stop reaching into BranchManager internals
Casting editorEngine.branches to any and poking branchMap bypasses branch-manager invariants and violates the “no any” rule. This will break the moment BranchManager’s internals change. Please expose a proper API on BranchManager (e.g. a public registerBranchData helper) and invoke that instead of reaching through a private map.
As per coding guidelines
🤖 Prompt for AI Agents
In packages/ai/src/tools/classes/create-multiple-branches.ts around lines
105-107, the code is directly accessing a private branchMap by casting
editorEngine.branches to any and setting an entry; stop reaching into
BranchManager internals and instead add a public API on BranchManager (e.g.,
registerBranchData(branchId: string, data: BranchData)) that encapsulates
adding/updating the map, export/update BranchManager types, then replace the
any-cast call with editorEngine.branches.registerBranchData(result.branch.id,
branchData); ensure the new method enforces invariants and update any call sites
and unit tests/type definitions accordingly.
Type of Change
Testing
Screenshots (if applicable)
Additional Notes
Important
Add
CreateMultipleBranchesToolto create multiple branches from a prompt, with integration into the toolset and API support for position overrides.CreateMultipleBranchesToolincreate-multiple-branches.tsto create multiple branches from a selected branch.CreateMultipleBranchesToolinindex.ts.CreateMultipleBranchesToolintoolset.tsundereditOnlyToolClasses.branch.tsto supportpositionOverridefor branch forking.This description was created by
for 44f15fd. You can customize this summary. It will automatically update as commits are pushed.
Summary by CodeRabbit
Release Notes