Skip to content

Commit af51106

Browse files
fix(issues): refuse stale edits and bump updated_at on chat detach
1 parent 0b66a4b commit af51106

5 files changed

Lines changed: 51 additions & 3 deletions

File tree

‎apps/sim/lib/issues/__integration__/issue-body-access.integration.ts‎

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -171,12 +171,21 @@ describe('issue bodies in PostgreSQL', () => {
171171
.where(eq(copilotChats.id, chatId))
172172
expect(chat.issueId).toBe(working[0].id)
173173

174+
const [before] = await db
175+
.select({ updatedAt: issue.updatedAt })
176+
.from(issue)
177+
.where(eq(issue.id, working[0].id))
174178
await db.transaction((tx) => detachChatFromIssuesInTx(tx, chatId))
175179
const [released] = await db
176-
.select({ status: issue.status, workingChatId: issue.workingChatId })
180+
.select({
181+
status: issue.status,
182+
workingChatId: issue.workingChatId,
183+
updatedAt: issue.updatedAt,
184+
})
177185
.from(issue)
178186
.where(eq(issue.id, working[0].id))
179-
expect(released).toEqual({ status: 'inbox', workingChatId: null })
187+
expect(released).toMatchObject({ status: 'inbox', workingChatId: null })
188+
expect(released.updatedAt.getTime()).toBeGreaterThan(before.updatedAt.getTime())
180189
const detached = await db
181190
.select({ kind: issueEvent.kind })
182191
.from(issueEvent)

‎apps/sim/lib/issues/application/issues.ts‎

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -325,7 +325,18 @@ export const updateIssue = defineAuthorizedWorkspaceUseCase({
325325
events.push({ kind: 'owner_changed', payload: { from: current.ownerId, to: input.ownerId } })
326326
}
327327
if (events.length > 0) {
328-
await transition(current.id, { statuses: ISSUE_STATUSES }, values, () => events, principal)
328+
const unchanged = {
329+
...(values.title === undefined ? {} : { title: current.title }),
330+
...(values.priority === undefined ? {} : { priority: current.priority }),
331+
...(values.ownerId === undefined ? {} : { ownerId: current.ownerId }),
332+
}
333+
await transition(
334+
current.id,
335+
{ statuses: ISSUE_STATUSES, unchanged },
336+
values,
337+
() => events,
338+
principal
339+
)
329340
}
330341
return { issue: await presentIssue(current.id) }
331342
},

‎apps/sim/lib/issues/repository.integration.ts‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -153,6 +153,22 @@ describe('issue state in PostgreSQL', () => {
153153
expect(await db.transaction((tx) => allocateIssueNumber(tx, 'org-b'))).toBe(1)
154154
})
155155

156+
it('refuses an edit when a field it changes no longer holds the value that was read', async () => {
157+
const db = testDb()
158+
const id = await insertIssue({ number: 11 })
159+
const rename = (from: string) =>
160+
db.transaction((tx) =>
161+
updateIssueInTx(
162+
tx,
163+
id,
164+
{ statuses: ['inbox'], unchanged: { title: from } },
165+
{ title: 'Renamed' }
166+
)
167+
)
168+
expect(await rename('Stale title')).toBeNull()
169+
expect((await rename('Title'))?.title).toBe('Renamed')
170+
})
171+
156172
it('lets only one of two transitions from the same state win', async () => {
157173
const db = testDb()
158174
const chatA = await newChat()

‎apps/sim/lib/issues/repository.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -144,6 +144,8 @@ export interface IssueStateGuard {
144144
statuses: readonly IssueStatus[]
145145
/** Inbox with a working chat is waiting for review; without one it is new. */
146146
hasWorkingChat?: boolean
147+
/** Fields that must still hold the values the caller read, so an edit never logs a stale "from". */
148+
unchanged?: { title?: string; priority?: number; ownerId?: string | null }
147149
}
148150

149151
/**
@@ -170,6 +172,15 @@ export async function updateIssueInTx(
170172
eq(issue.id, issueId),
171173
inArray(issue.status, [...guard.statuses]),
172174
chatCondition,
175+
guard.unchanged?.title === undefined ? undefined : eq(issue.title, guard.unchanged.title),
176+
guard.unchanged?.priority === undefined
177+
? undefined
178+
: eq(issue.priority, guard.unchanged.priority),
179+
guard.unchanged?.ownerId === undefined
180+
? undefined
181+
: guard.unchanged.ownerId === null
182+
? isNull(issue.ownerId)
183+
: eq(issue.ownerId, guard.unchanged.ownerId),
173184
isNull(issue.deletedAt)
174185
)
175186
)

‎packages/db/migrations/0400_issue.sql‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -182,6 +182,7 @@ BEGIN
182182
IF NEW.status = 'in_progress' THEN
183183
NEW.status := 'inbox';
184184
END IF;
185+
NEW.updated_at := now();
185186
INSERT INTO issue_event (id, issue_id, kind, payload)
186187
VALUES (gen_random_uuid()::text, NEW.id, 'chat_detached', jsonb_build_object('chatId', OLD.working_chat_id));
187188
END IF;

0 commit comments

Comments
 (0)