Skip to content

Commit 93fbf58

Browse files
authored
feat(chat): soft-delete sidebar chats with restore from Recently Deleted (#5830)
* feat(chat): soft-delete sidebar chats with restore from Recently Deleted * fix(chat): review round 1 — restore workspace authz, purge recheck, archived-list invalidation * test(chat): update SSE handler assertions for workspaceLists invalidation * fix(chat): bump updatedAt on restore, recheck retention cutoff in task cleanup * fix(chat): drop explicit feedback delete in task cleanup — chat FK cascade covers it * test(chat): cover restore route; guard legacy copilot delete from hard-deleting mothership chats * chore: revert unintended bun.lock drift from worktree install * fix(cleanup): recheck workflow archive cutoff on delete; export deletedAt in chat drain
1 parent 943d184 commit 93fbf58

35 files changed

Lines changed: 18069 additions & 113 deletions

File tree

apps/sim/app/api/copilot/chat/delete/route.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,16 @@ export const DELETE = withRouteHandler(async (request: NextRequest) => {
3535
return NextResponse.json({ success: true })
3636
}
3737

38+
// Mothership (sidebar) chats are soft-deleted through their own route so
39+
// they land in Recently Deleted — this legacy hard-delete must not bypass
40+
// that.
41+
if (chat.type === 'mothership') {
42+
return NextResponse.json(
43+
{ success: false, error: 'Use the mothership chat delete endpoint' },
44+
{ status: 400 }
45+
)
46+
}
47+
3848
const [deleted] = await db
3949
.delete(copilotChats)
4050
.where(and(eq(copilotChats.id, parsed.chatId), eq(copilotChats.userId, session.user.id)))

apps/sim/app/api/copilot/chat/queries.ts

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { copilotChats } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { authorizeWorkflowByWorkspacePermission } from '@sim/platform-authz/workflow'
55
import { toError } from '@sim/utils/errors'
6-
import { and, desc, eq } from 'drizzle-orm'
6+
import { and, desc, eq, isNull } from 'drizzle-orm'
77
import { type NextRequest, NextResponse } from 'next/server'
88
import { getLatestRunForStream } from '@/lib/copilot/async-runs/repository'
99
import { buildEffectiveChatTranscript } from '@/lib/copilot/chat/effective-transcript'
@@ -189,7 +189,13 @@ export async function GET(req: NextRequest) {
189189
updatedAt: copilotChats.updatedAt,
190190
})
191191
.from(copilotChats)
192-
.where(and(eq(copilotChats.userId, authenticatedUserId), scopeFilter))
192+
.where(
193+
and(
194+
eq(copilotChats.userId, authenticatedUserId),
195+
isNull(copilotChats.deletedAt),
196+
scopeFilter
197+
)
198+
)
193199
.orderBy(desc(copilotChats.updatedAt))
194200

195201
const scope = workflowId ? `workflow ${workflowId}` : `workspace ${workspaceId}`

apps/sim/app/api/copilot/chat/resources/route.ts

Lines changed: 36 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { db } from '@sim/db'
22
import { copilotChats } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
4-
import { and, eq, sql } from 'drizzle-orm'
4+
import { and, eq, isNull, sql } from 'drizzle-orm'
55
import { type NextRequest, NextResponse } from 'next/server'
66
import {
77
addCopilotChatResourceContract,
@@ -64,7 +64,13 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
6464
const [chat] = await db
6565
.select({ resources: copilotChats.resources })
6666
.from(copilotChats)
67-
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
67+
.where(
68+
and(
69+
eq(copilotChats.id, chatId),
70+
eq(copilotChats.userId, userId),
71+
isNull(copilotChats.deletedAt)
72+
)
73+
)
6874
.limit(1)
6975

7076
if (!chat) {
@@ -91,7 +97,13 @@ export const POST = withRouteHandler(async (req: NextRequest) => {
9197
await db
9298
.update(copilotChats)
9399
.set({ resources: sql`${JSON.stringify(merged)}::jsonb`, updatedAt: new Date() })
94-
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
100+
.where(
101+
and(
102+
eq(copilotChats.id, chatId),
103+
eq(copilotChats.userId, userId),
104+
isNull(copilotChats.deletedAt)
105+
)
106+
)
95107

96108
logger.info('Added resource to chat', { chatId, resource })
97109

@@ -124,7 +136,13 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => {
124136
const [chat] = await db
125137
.select({ resources: copilotChats.resources })
126138
.from(copilotChats)
127-
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
139+
.where(
140+
and(
141+
eq(copilotChats.id, chatId),
142+
eq(copilotChats.userId, userId),
143+
isNull(copilotChats.deletedAt)
144+
)
145+
)
128146
.limit(1)
129147

130148
if (!chat) {
@@ -142,7 +160,13 @@ export const PATCH = withRouteHandler(async (req: NextRequest) => {
142160
await db
143161
.update(copilotChats)
144162
.set({ resources: sql`${JSON.stringify(newOrder)}::jsonb`, updatedAt: new Date() })
145-
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
163+
.where(
164+
and(
165+
eq(copilotChats.id, chatId),
166+
eq(copilotChats.userId, userId),
167+
isNull(copilotChats.deletedAt)
168+
)
169+
)
146170

147171
logger.info('Reordered resources for chat', { chatId, count: newOrder.length })
148172

@@ -182,7 +206,13 @@ export const DELETE = withRouteHandler(async (req: NextRequest) => {
182206
), '[]'::jsonb)`,
183207
updatedAt: new Date(),
184208
})
185-
.where(and(eq(copilotChats.id, chatId), eq(copilotChats.userId, userId)))
209+
.where(
210+
and(
211+
eq(copilotChats.id, chatId),
212+
eq(copilotChats.userId, userId),
213+
isNull(copilotChats.deletedAt)
214+
)
215+
)
186216
.returning({ resources: copilotChats.resources })
187217

188218
if (!updated) {

apps/sim/app/api/copilot/chat/update-messages/route.test.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,6 +46,7 @@ vi.mock('@/lib/copilot/chat/messages-store', () => ({
4646
vi.mock('drizzle-orm', () => ({
4747
and: vi.fn((...conditions: unknown[]) => ({ conditions, type: 'and' })),
4848
eq: vi.fn((field: unknown, value: unknown) => ({ field, value, type: 'eq' })),
49+
isNull: vi.fn((field: unknown) => ({ field, type: 'isNull' })),
4950
}))
5051

5152
import { POST } from '@/app/api/copilot/chat/update-messages/route'

apps/sim/app/api/copilot/chats/route.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ export const GET = withRouteHandler(async (_request: NextRequest) => {
6262
.where(
6363
and(
6464
eq(copilotChats.userId, userId),
65+
isNull(copilotChats.deletedAt),
6566
or(
6667
and(isNull(copilotChats.workflowId), isNull(copilotChats.workspaceId)),
6768
inAccessibleWorkspace

apps/sim/app/api/mothership/chats/[chatId]/fork/route.test.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,18 @@ vi.mock('@sim/db/schema', () => ({
7070
previewYaml: 'copilotChats.previewYaml',
7171
planArtifact: 'copilotChats.planArtifact',
7272
config: 'copilotChats.config',
73+
deletedAt: 'copilotChats.deletedAt',
7374
},
7475
workspaceFiles: {
7576
id: 'workspaceFiles.id',
7677
},
7778
}))
7879

7980
vi.mock('drizzle-orm', () => ({
81+
and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
8082
eq: vi.fn((field: unknown, value: unknown) => ({ type: 'eq', field, value })),
8183
inArray: vi.fn((field: unknown, values: unknown) => ({ type: 'inArray', field, values })),
84+
isNull: vi.fn((field: unknown) => ({ type: 'isNull', field })),
8285
}))
8386

8487
vi.mock('@/lib/copilot/resources/persistence', () => ({

apps/sim/app/api/mothership/chats/[chatId]/fork/route.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { db } from '@sim/db'
22
import { copilotChats, workspaceFiles } from '@sim/db/schema'
33
import { createLogger } from '@sim/logger'
44
import { generateId } from '@sim/utils/id'
5-
import { eq, inArray } from 'drizzle-orm'
5+
import { and, eq, inArray, isNull } from 'drizzle-orm'
66
import { type NextRequest, NextResponse } from 'next/server'
77
import { forkMothershipChatContract } from '@/lib/api/contracts/mothership-chats'
88
import { parseRequest } from '@/lib/api/server'
@@ -84,7 +84,7 @@ export const POST = withRouteHandler(
8484
config: copilotChats.config,
8585
})
8686
.from(copilotChats)
87-
.where(eq(copilotChats.id, chatId))
87+
.where(and(eq(copilotChats.id, chatId), isNull(copilotChats.deletedAt)))
8888
.limit(1)
8989

9090
if (!parent || parent.userId !== userId || parent.type !== 'mothership') {
Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
/**
2+
* @vitest-environment node
3+
*/
4+
import { copilotHttpMock, copilotHttpMockFns } from '@sim/testing'
5+
import { NextRequest } from 'next/server'
6+
import { beforeEach, describe, expect, it, vi } from 'vitest'
7+
8+
const {
9+
mockDbSelect,
10+
mockSelectFrom,
11+
mockSelectWhere,
12+
mockSelectLimit,
13+
mockDbUpdate,
14+
mockDbSet,
15+
mockUpdateWhere,
16+
mockDbReturning,
17+
mockAssertActiveWorkspaceAccess,
18+
mockPublishStatusChanged,
19+
} = vi.hoisted(() => ({
20+
mockDbSelect: vi.fn(),
21+
mockSelectFrom: vi.fn(),
22+
mockSelectWhere: vi.fn(),
23+
mockSelectLimit: vi.fn(),
24+
mockDbUpdate: vi.fn(),
25+
mockDbSet: vi.fn(),
26+
mockUpdateWhere: vi.fn(),
27+
mockDbReturning: vi.fn(),
28+
mockAssertActiveWorkspaceAccess: vi.fn(),
29+
mockPublishStatusChanged: vi.fn(),
30+
}))
31+
32+
vi.mock('@sim/db', () => ({
33+
db: {
34+
select: mockDbSelect,
35+
update: mockDbUpdate,
36+
},
37+
}))
38+
39+
vi.mock('@sim/db/schema', () => ({
40+
copilotChats: {
41+
id: 'copilotChats.id',
42+
userId: 'copilotChats.userId',
43+
type: 'copilotChats.type',
44+
workspaceId: 'copilotChats.workspaceId',
45+
updatedAt: 'copilotChats.updatedAt',
46+
lastSeenAt: 'copilotChats.lastSeenAt',
47+
deletedAt: 'copilotChats.deletedAt',
48+
},
49+
}))
50+
51+
vi.mock('drizzle-orm', () => ({
52+
and: vi.fn((...conditions: unknown[]) => ({ type: 'and', conditions })),
53+
eq: vi.fn((field: unknown, value: unknown) => ({ type: 'eq', field, value })),
54+
isNotNull: vi.fn((field: unknown) => ({ type: 'isNotNull', field })),
55+
}))
56+
57+
vi.mock('@/lib/copilot/request/http', () => ({
58+
...copilotHttpMock,
59+
createForbiddenResponse: vi.fn((message: string) => ({
60+
status: 403,
61+
ok: false,
62+
json: async () => ({ error: message }),
63+
})),
64+
}))
65+
66+
vi.mock('@/lib/workspaces/permissions/utils', () => ({
67+
assertActiveWorkspaceAccess: mockAssertActiveWorkspaceAccess,
68+
isWorkspaceAccessDeniedError: (error: unknown) =>
69+
error instanceof Error && error.message === 'ACCESS_DENIED',
70+
}))
71+
72+
vi.mock('@/lib/copilot/chat-status', () => ({
73+
chatPubSub: { publishStatusChanged: mockPublishStatusChanged },
74+
}))
75+
76+
vi.mock('@/lib/posthog/server', () => ({
77+
captureServerEvent: vi.fn(),
78+
}))
79+
80+
import { POST } from '@/app/api/mothership/chats/[chatId]/restore/route'
81+
82+
function makeRequest(chatId: string) {
83+
return new NextRequest(`http://localhost:3000/api/mothership/chats/${chatId}/restore`, {
84+
method: 'POST',
85+
})
86+
}
87+
88+
function makeContext(chatId: string) {
89+
return { params: Promise.resolve({ chatId }) }
90+
}
91+
92+
describe('POST /api/mothership/chats/[chatId]/restore', () => {
93+
beforeEach(() => {
94+
vi.clearAllMocks()
95+
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValue({
96+
userId: 'user-1',
97+
isAuthenticated: true,
98+
})
99+
mockSelectLimit.mockResolvedValue([{ workspaceId: 'workspace-1' }])
100+
mockSelectWhere.mockReturnValue({ limit: mockSelectLimit })
101+
mockSelectFrom.mockReturnValue({ where: mockSelectWhere })
102+
mockDbSelect.mockReturnValue({ from: mockSelectFrom })
103+
mockDbReturning.mockResolvedValue([{ workspaceId: 'workspace-1' }])
104+
mockUpdateWhere.mockReturnValue({ returning: mockDbReturning })
105+
mockDbSet.mockReturnValue({ where: mockUpdateWhere })
106+
mockDbUpdate.mockReturnValue({ set: mockDbSet })
107+
mockAssertActiveWorkspaceAccess.mockResolvedValue(undefined)
108+
})
109+
110+
it('returns 401 when unauthenticated', async () => {
111+
copilotHttpMockFns.mockAuthenticateCopilotRequestSessionOnly.mockResolvedValueOnce({
112+
userId: null,
113+
isAuthenticated: false,
114+
})
115+
116+
const response = await POST(makeRequest('chat-1'), makeContext('chat-1'))
117+
expect(response.status).toBe(401)
118+
expect(mockDbUpdate).not.toHaveBeenCalled()
119+
})
120+
121+
it('returns 404 when no soft-deleted chat is owned by the caller', async () => {
122+
mockSelectLimit.mockResolvedValueOnce([])
123+
124+
const response = await POST(makeRequest('chat-missing'), makeContext('chat-missing'))
125+
expect(response.status).toBe(404)
126+
expect(mockAssertActiveWorkspaceAccess).not.toHaveBeenCalled()
127+
expect(mockDbUpdate).not.toHaveBeenCalled()
128+
})
129+
130+
it('returns 403 when the caller lost access to the workspace', async () => {
131+
mockAssertActiveWorkspaceAccess.mockRejectedValueOnce(new Error('ACCESS_DENIED'))
132+
133+
const response = await POST(makeRequest('chat-1'), makeContext('chat-1'))
134+
expect(response.status).toBe(403)
135+
expect(mockDbUpdate).not.toHaveBeenCalled()
136+
})
137+
138+
it('restores the chat, bumping updatedAt and lastSeenAt, and publishes the event', async () => {
139+
const response = await POST(makeRequest('chat-1'), makeContext('chat-1'))
140+
141+
expect(response.status).toBe(200)
142+
expect(await response.json()).toEqual({ success: true })
143+
expect(mockAssertActiveWorkspaceAccess).toHaveBeenCalledWith('workspace-1', 'user-1')
144+
expect(mockDbSet).toHaveBeenCalledWith({
145+
deletedAt: null,
146+
updatedAt: expect.any(Date),
147+
lastSeenAt: expect.any(Date),
148+
})
149+
expect(mockPublishStatusChanged).toHaveBeenCalledWith({
150+
workspaceId: 'workspace-1',
151+
chatId: 'chat-1',
152+
type: 'created',
153+
})
154+
})
155+
156+
it('returns 404 when the chat is restored concurrently before the update lands', async () => {
157+
mockDbReturning.mockResolvedValueOnce([])
158+
159+
const response = await POST(makeRequest('chat-1'), makeContext('chat-1'))
160+
expect(response.status).toBe(404)
161+
expect(mockPublishStatusChanged).not.toHaveBeenCalled()
162+
})
163+
})

0 commit comments

Comments
 (0)