Skip to content

Commit 081b5cf

Browse files
waleedlatif1claude
andcommitted
fix(copilot): hold a reorder for pending deletes too
The reorder gate landed one case short: a delete that has not reached the server leaves the server holding a resource the client's order omits, so the order fails its identity check exactly as an unlanded add does. Gating only on unpersisted adds let that order fire and be discarded as unsatisfiable, losing the tab order until the next reorder or hydration. Name the predicate for what it actually decides — whether a pending write changes WHICH resources the chat holds — and cover both directions. A failing update to an already-stored resource still does not park the order, which is what the gate was narrowed for. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FwmaLmAXSK2hsPBGZmkPnT
1 parent 390e1b3 commit 081b5cf

3 files changed

Lines changed: 37 additions & 13 deletions

File tree

apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1707,7 +1707,7 @@ export function useChat(
17071707
while (true) {
17081708
const pendingOrder = pendingResourceReordersRef.current.get(chatId)
17091709
if (!pendingOrder) return
1710-
if (resourcePersistenceQueue.hasUnpersistedWrites(chatId)) return
1710+
if (resourcePersistenceQueue.hasPendingIdentityChanges(chatId)) return
17111711

17121712
const inFlightWrites = resourcePersistenceQueue.getInFlightWrites(chatId)
17131713
if (inFlightWrites.length > 0) {

apps/sim/lib/copilot/resources/client-persistence-queue.test.ts

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,27 @@ describe('ResourcePersistenceQueue', () => {
219219
expect(queue.getPendingResourceKeys('chat-1')).toEqual(new Set())
220220
})
221221

222-
it('does not report an unpersisted write once the resource reaches the server', async () => {
222+
it('reports a pending identity change while a deletion has not landed', async () => {
223+
const persist = vi
224+
.fn<(chatId: string, update: MothershipResourceUpdate) => Promise<unknown>>()
225+
.mockResolvedValue({ success: true })
226+
const remove = vi.fn<() => Promise<unknown>>().mockRejectedValueOnce(new Error('offline'))
227+
const queue = new ResourcePersistenceQueue({ persist, onError })
228+
229+
queue.enqueue(TABLE_RESOURCE, 'chat-1', 'chat-1')
230+
await Promise.allSettled(queue.getInFlightWrites('chat-1'))
231+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(false)
232+
233+
// The server still holds a resource the client has dropped, so an order
234+
// built from client state would not match and must wait for the delete.
235+
const removal = queue.remove(TABLE_RESOURCE.type, TABLE_RESOURCE.id, 'chat-1')
236+
removal.scheduleDelete('chat-1', remove)
237+
await vi.waitFor(() => expect(onError).toHaveBeenCalledOnce())
238+
239+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(true)
240+
})
241+
242+
it('does not report an identity change for a failing update to a stored resource', async () => {
223243
const persist = vi
224244
.fn<(chatId: string, update: MothershipResourceUpdate) => Promise<unknown>>()
225245
.mockResolvedValueOnce({ success: true })
@@ -228,17 +248,17 @@ describe('ResourcePersistenceQueue', () => {
228248

229249
queue.enqueue(TABLE_RESOURCE, 'chat-1', 'chat-1')
230250
await Promise.allSettled(queue.getInFlightWrites('chat-1'))
231-
expect(queue.hasUnpersistedWrites('chat-1')).toBe(false)
251+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(false)
232252

233253
// A pin update for the same, already-stored resource keeps failing. The
234254
// resource is on the server, so a reorder naming it stays valid.
235255
queue.enqueue({ ...TABLE_RESOURCE, viewId: 'view-a' }, 'chat-1', 'chat-1')
236256
await vi.waitFor(() => expect(onError).toHaveBeenCalledOnce())
237257
expect(queue.getPendingResourceKeys('chat-1')).toEqual(new Set(['table:table-1']))
238-
expect(queue.hasUnpersistedWrites('chat-1')).toBe(false)
258+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(false)
239259

240260
await queue.flush('chat-1')
241-
expect(queue.hasUnpersistedWrites('chat-1')).toBe(false)
261+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(false)
242262
})
243263

244264
it('reports an unpersisted write while a first add has never succeeded', async () => {
@@ -250,7 +270,7 @@ describe('ResourcePersistenceQueue', () => {
250270
queue.enqueue(TABLE_RESOURCE, 'chat-1', 'chat-1')
251271
await vi.waitFor(() => expect(onError).toHaveBeenCalledOnce())
252272

253-
expect(queue.hasUnpersistedWrites('chat-1')).toBe(true)
273+
expect(queue.hasPendingIdentityChanges('chat-1')).toBe(true)
254274
})
255275

256276
it('keeps the newer chat-scoped update when a provisional scope is adopted', async () => {

apps/sim/lib/copilot/resources/client-persistence-queue.ts

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -117,15 +117,19 @@ export class ResourcePersistenceQueue {
117117
}
118118

119119
/**
120-
* Whether a pending write would add a resource the server does not store yet.
120+
* Whether a pending write would change which resources the chat holds — an
121+
* add the server has not accepted yet, or a delete that has not landed.
121122
*
122-
* Only these may hold a reorder back — the server rejects an order naming a
123-
* resource it has never seen. A pending UPDATE to a resource it already
124-
* stores (a saved-view pin) must not: that write can fail indefinitely, and
125-
* gating on it would park tab ordering for the rest of the session.
123+
* Only these may hold a reorder back, because only these make the client's
124+
* identity set disagree with the server's, and the server validates a reorder
125+
* against exactly that. A pending UPDATE to a resource it already stores (a
126+
* saved-view pin) must NOT: such a write can fail indefinitely, and gating on
127+
* it would park tab ordering for the rest of the session.
126128
*/
127-
hasUnpersistedWrites(scopeId: string): boolean {
128-
return this.getScopedKeys(this.pendingKeys, scopeId).some((key) => !this.persistedKeys.has(key))
129+
hasPendingIdentityChanges(scopeId: string): boolean {
130+
return this.getScopedKeys(this.pendingKeys, scopeId).some(
131+
(key) => !this.persistedKeys.has(key) || this.pendingRemovals.has(key)
132+
)
129133
}
130134

131135
getPendingResourceKeys(scopeId: string): Set<string> {

0 commit comments

Comments
 (0)