Skip to content

Commit 5530dd0

Browse files
committed
fix(chat): hold a steering message a live run has not accepted yet
A read can land between steerRun recording the message and the worker accepting it, and read 'unknown' then. Dropping the record there left a message the model would still apply with no row. 'unknown' now reads as held while Sim's run is live, as 'queued' does, and as not held once it ended, so nothing is stranded: the sender reads again after the turn ends. A message the user dismissed while a turn still held it shows as not delivered again if that turn leaves it, rather than going out as a turn on its own.
1 parent 978e26f commit 5530dd0

6 files changed

Lines changed: 103 additions & 17 deletions

File tree

‎apps/sim/app/api/copilot/chat/steer/disposition/route.test.ts‎

Lines changed: 28 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -107,17 +107,30 @@ describe('reading or dismissing a steering message', () => {
107107
})
108108
})
109109

110-
it('reads a message a live run never received as not delivered, without delivering it', async () => {
110+
it('holds a message a live run has not accepted yet, since its steer may be on its way', async () => {
111111
recorded()
112112
worker.answers.push(Response.json({ steeringId: STEERING_ID, status: 'unknown' }))
113113

114114
const response = await read()
115115

116-
expect(await response.json()).toEqual({ steeringId: STEERING_ID, status: 'not_applied' })
116+
expect(await response.json()).toEqual({ steeringId: STEERING_ID, status: 'queued' })
117117
expect(worker.calls).toEqual([READ])
118118
expect(mockRecordDisposition).not.toHaveBeenCalled()
119119
})
120120

121+
it('reads a message a run ended without accepting as not delivered, for good', async () => {
122+
recorded()
123+
runIs('complete')
124+
worker.answers.push(Response.json({ steeringId: STEERING_ID, status: 'unknown' }))
125+
126+
const response = await read()
127+
128+
expect(await response.json()).toEqual({ steeringId: STEERING_ID, status: 'not_applied' })
129+
expect(mockRecordDisposition).toHaveBeenCalledWith('chat-1', [STEERING_ID], {
130+
status: 'not_applied',
131+
})
132+
})
133+
121134
it('reads an id the chat holds no record of as not delivered, asking the worker nothing', async () => {
122135
const response = await read()
123136

@@ -155,8 +168,20 @@ describe('reading or dismissing a steering message', () => {
155168
}
156169
)
157170

158-
it('never delivers a message it dismisses, even one a live run never received', async () => {
171+
it('keeps a message a live run has not accepted yet, and never delivers it', async () => {
172+
recorded()
173+
worker.answers.push(Response.json({ steeringId: STEERING_ID, status: 'unknown' }))
174+
175+
const response = await dismiss()
176+
177+
expect(await response.json()).toEqual({ status: 'busy', streamId: TURN_ID })
178+
expect(worker.calls).toEqual([READ])
179+
expect(dbChainMockFns.delete).not.toHaveBeenCalled()
180+
})
181+
182+
it('dismisses a message once its run ended without accepting it', async () => {
159183
recorded()
184+
runIs('complete')
160185
worker.answers.push(Response.json({ steeringId: STEERING_ID, status: 'unknown' }))
161186

162187
const response = await dismiss()

‎apps/sim/app/api/copilot/chat/steer/route.test.ts‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -336,8 +336,17 @@ describe('POST /api/copilot/chat/steer', () => {
336336
expect(mockAppend).not.toHaveBeenCalled()
337337
})
338338

339-
it('steers it afresh once no run holds it', async () => {
339+
it('waits on a live run that has not accepted it yet, never sending it twice', async () => {
340340
worker.read.push(Response.json({ steeringId: STEERING_ID, status: 'unknown' }))
341+
342+
const response = await POST(steerRequest())
343+
344+
expect(await response.json()).toEqual({ steeringId: STEERING_ID, status: 'queued' })
345+
expect(worker.calls).toEqual([READ])
346+
})
347+
348+
it('steers it afresh once no run holds it', async () => {
349+
worker.read.push(Response.json({ steeringId: STEERING_ID, status: 'not_applied' }))
341350
worker.v2.push(Response.json({ steeringId: STEERING_ID, status: 'queued' }))
342351

343352
const response = await POST(steerRequest())

‎apps/sim/app/workspace/[workspaceId]/home/hooks/use-chat.dom.test.tsx‎

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4062,6 +4062,24 @@ describe('useChat remount send recovery', () => {
40624062
expect(deliveryOf(view, id)).not.toBe('failed')
40634063
})
40644064

4065+
it('shows a dismissed message held by a turn as not delivered once that turn leaves it', async () => {
4066+
state.dismissAnswers = [{ status: 'busy', streamId: 'earlier-turn' }]
4067+
state.steerDispositions = ['not_applied']
4068+
const view = await restoredChat([failedEntry])
4069+
4070+
await act(async () => {
4071+
view.getResult().dismissMessage(id)
4072+
await settle()
4073+
})
4074+
await waitFor(() => state.readBodies.length === 1)
4075+
await act(async () => {
4076+
await settle()
4077+
})
4078+
4079+
expect(deliveryOf(view, id)).toBe('failed')
4080+
expect(state.postBodies).toEqual([])
4081+
})
4082+
40654083
it('stays not delivered when the server gave no answer', async () => {
40664084
state.dismissAnswers = [500]
40674085
const view = await restoredChat([failedEntry])

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4787,11 +4787,14 @@ export function useChat(
47874787
throw error
47884788
})
47894789
if (outcome.status === 'busy') {
4790+
/* The user gave it up: if that turn ends without applying it, it shows as
4791+
not delivered again rather than going out as a turn on its own. */
47904792
updateOutgoing(id, {
47914793
status: 'steered',
47924794
streamId: outcome.streamId,
47934795
attempts: 0,
47944796
reads: 0,
4797+
noAutoSend: true,
47954798
})
47964799
void deliverOutgoing()
47974800
return

‎apps/sim/lib/mothership/chat/application/steering-fallback.integration.ts‎

Lines changed: 40 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -490,16 +490,32 @@ describe('steering fallback gate in PostgreSQL', () => {
490490
expect(await storedSteering(chatId, steeringId)).toEqual({ streamId: laterTurn })
491491
})
492492

493-
it('admits a turn for a message a live run never received, without delivering it', async () => {
494-
const { chatId, turnId, steeringId } = await chatWithSteeredTurn()
495-
await runFor(chatId, turnId, 'active')
496-
worker.answers.push(Response.json({ steeringId, status: 'unknown' }))
493+
it('holds a message a live run has not accepted yet, and admits it once the run ended', async () => {
494+
const live = await chatWithSteeredTurn()
495+
await runFor(live.chatId, live.turnId, 'active')
496+
const ended = await chatWithSteeredTurn()
497+
await runFor(ended.chatId, ended.turnId, 'complete')
498+
worker.answers.push(
499+
Response.json({ steeringId: live.steeringId, status: 'unknown' }),
500+
Response.json({ steeringId: ended.steeringId, status: 'unknown' })
501+
)
497502

498-
expect(await resolveSteeringFallback({ chatId, messageId: steeringId, userId })).toEqual({
499-
kind: 'send',
503+
const whileLive = await resolveSteeringFallback({
504+
chatId: live.chatId,
505+
messageId: live.steeringId,
506+
userId,
500507
})
501-
expect(worker.calls).toEqual([READ])
502-
expect(await transcriptIds(chatId)).not.toContain(steeringId)
508+
const onceEnded = await resolveSteeringFallback({
509+
chatId: ended.chatId,
510+
messageId: ended.steeringId,
511+
userId,
512+
})
513+
514+
expect(whileLive).toEqual({ kind: 'busy', streamId: live.turnId })
515+
expect(await transcriptIds(live.chatId)).toContain(live.steeringId)
516+
expect(onceEnded).toEqual({ kind: 'send' })
517+
expect(await transcriptIds(ended.chatId)).not.toContain(ended.steeringId)
518+
expect(worker.calls).toEqual([READ, READ])
503519
})
504520

505521
it("decides from Sim's run with a worker that predates the read, never re-sending", async () => {
@@ -580,7 +596,7 @@ describe('steering fallback gate in PostgreSQL', () => {
580596
expect(await transcriptIds(chatId)).toContain(turnId)
581597
})
582598

583-
it('never delivers a message it dismisses, even one a live run never received', async () => {
599+
it('keeps a message a live run has not accepted yet, and never delivers it', async () => {
584600
const { chatId, turnId, steeringId } = await chatWithSteeredTurn()
585601
await runFor(chatId, turnId, 'active')
586602
worker.answers.push(Response.json({ steeringId, status: 'unknown' }))
@@ -590,8 +606,22 @@ describe('steering fallback gate in PostgreSQL', () => {
590606
input: { chatId, messageId: steeringId },
591607
})
592608

593-
expect(answer).toEqual({ status: 'dismissed' })
609+
expect(answer).toEqual({ status: 'busy', streamId: turnId })
594610
expect(worker.calls).toEqual([READ])
611+
expect(await transcriptIds(chatId)).toContain(steeringId)
612+
})
613+
614+
it('dismisses a message once its run ended without accepting it', async () => {
615+
const { chatId, turnId, steeringId } = await chatWithSteeredTurn()
616+
await runFor(chatId, turnId, 'complete')
617+
worker.answers.push(Response.json({ steeringId, status: 'unknown' }))
618+
619+
const answer = await dismissSteeringMessage.execute({
620+
principal: owner,
621+
input: { chatId, messageId: steeringId },
622+
})
623+
624+
expect(answer).toEqual({ status: 'dismissed' })
595625
expect(await transcriptIds(chatId)).not.toContain(steeringId)
596626
})
597627

‎apps/sim/lib/mothership/chat/application/steering-fallback.ts‎

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,9 @@ export async function steeredRunHasEnded(streamId: string, userId: string): Prom
106106
* can deliver it. A final answer is kept on the record and answers later reads. A
107107
* `queued` answer counts only while Sim still runs that turn: a run Sim recorded as
108108
* ended (one whose worker owner died, say) never applies it, and waiting on it
109-
* would hold the message forever.
109+
* would hold the message forever. `unknown` (nothing accepted yet) is held the same
110+
* way: the steer that recorded it may still be on its way to the worker, and a
111+
* record dropped under it would leave a message the model has with no row.
110112
*
111113
* A worker without the read decides from Sim's run alone, never by re-sending the
112114
* message: a live run may hold it, and an ended one can no longer apply it. If it
@@ -133,8 +135,7 @@ export async function readSteeringRecord(params: {
133135
? { status: 'not_held' }
134136
: { status: 'held' }
135137
}
136-
if (answer.status === 'unknown') return { status: 'not_held' }
137-
if (answer.status === 'queued') {
138+
if (answer.status === 'queued' || answer.status === 'unknown') {
138139
if (!(await steeredRunHasEnded(record.streamId, userId))) return { status: 'held' }
139140
await recordSteeringDisposition(chatId, [messageId], { status: 'not_applied' })
140141
return { status: 'not_held' }

0 commit comments

Comments
 (0)