Feat/persist resources (WIP)#3553
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
PR SummaryMedium Risk Overview Adds a dedicated Updates the task UI to support adding resources via a + dropdown, closing tabs, and drag-and-drop reordering, and refactors resource rendering via a new Integrates resource persistence into execution: tool results can yield resources (including via VFS Written by Cursor Bugbot for commit 847c899. This will update automatically on new commits. Configure here. |
...ace/[workspaceId]/home/components/mothership-view/components/resource-tabs/resource-tabs.tsx
Outdated
Show resolved
Hide resolved
...app/workspace/[workspaceId]/home/components/mothership-view/components/resource-registry.tsx
Outdated
Show resolved
Hide resolved
| error: err instanceof Error ? err.message : String(err), | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Race condition in read-modify-write resource persistence
Medium Severity
persistChatResources performs a non-atomic read-modify-write (SELECT then UPDATE) on the resources JSONB column. It's called fire-and-forget via .catch() from the tool execution handler. If two tool results complete in quick succession, the second persist can start its SELECT before the first's UPDATE completes, causing the first resource addition to be silently lost from the database.
Additional Locations (1)
Greptile SummaryThis PR replaces the previous heuristic client-side resource extraction (parsing SSE payloads) with authoritative server-side persistence: tool results now emit Key observations:
Confidence Score: 3/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant Client as Browser (useChat)
participant SSE as SSE Stream
participant ToolExec as tool-execution.ts
participant DB as Database (copilotChats)
participant API as /api/copilot/chat/resources
Client->>SSE: sendMessage()
SSE->>ToolExec: executeToolAndReport()
ToolExec->>ToolExec: extractResourcesFromToolResult()
ToolExec-->>DB: persistChatResources() [fire-and-forget]
ToolExec-->>SSE: emit resource_added event
SSE-->>Client: { type: 'resource_added', resource }
Client->>Client: addResource() → setResources()
Note over Client,DB: User manually adds/removes/reorders
Client->>API: POST /resources (add)
API->>DB: UPDATE copilotChats SET resources = merged
API-->>Client: { success, resources }
Client->>API: DELETE /resources (remove)
API->>DB: UPDATE copilotChats SET resources = filtered
API-->>Client: { success, resources }
Client->>API: PATCH /resources (reorder)
API->>DB: UPDATE copilotChats SET resources = newOrder
API-->>Client: { success, resources }
Note over Client,DB: On page reload
Client->>API: GET /api/copilot/chat?chatId=...
API->>DB: SELECT resources FROM copilotChats
DB-->>API: resources[]
API-->>Client: chatHistory.resources
Client->>Client: setResources(chatHistory.resources)
|
| return [] | ||
| } | ||
|
|
||
| case 'knowledge': { |
There was a problem hiding this comment.
Missing updatedAt on persistence write
persistChatResources updates the resources column but omits updatedAt, making it inconsistent with all three handlers in route.ts (POST/PATCH/DELETE) which explicitly set updatedAt: new Date(). Any updatedAt-based ordering or staleness logic will not reflect server-side resource mutations.
| return [] | |
| } | |
| case 'knowledge': { | |
| await db | |
| .update(copilotChats) | |
| .set({ resources: sql`${JSON.stringify(merged)}::jsonb`, updatedAt: new Date() }) | |
| .where(eq(copilotChats.id, chatId)) |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
There are 2 total unresolved issues (including 1 from previous review).
Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.


Summary
Allows people to create and delete resources.
Type of Change
Testing
How has this been tested? What should reviewers focus on?
Checklist
Screenshots/Videos