mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 08:02:38 +00:00
fix(ai-sessions): refuse a held run before mutating, and keep an in-flight draft
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
51f3d86bf0
commit
fac221c1bb
@@ -3990,6 +3990,15 @@ export class AIChatManager {
|
||||
throw new Error('No user message found at the specified index')
|
||||
}
|
||||
|
||||
// Refused here rather than at the send below, which is reached only after the
|
||||
// truncation has already rewound this tab's transcript: the driver's turn-end
|
||||
// re-read would put it back, but until then the watcher sits on a conversation
|
||||
// that lost its tail for a resend that never ran.
|
||||
if (this.runHeldElsewhere) {
|
||||
sendUserToast('This session is running in another tab. Retry once it finishes.', true)
|
||||
return
|
||||
}
|
||||
|
||||
// Resolve the API restart point BEFORE reserving bytes or truncating: a
|
||||
// stale index must fail while nothing has been mutated, or the transcript
|
||||
// would be left truncated with the reservation leaked. A negative index
|
||||
|
||||
@@ -167,6 +167,11 @@ export async function withSessionRunLock<T>(
|
||||
// tab holds it, which is exactly the "refuse, don't stack up turns"
|
||||
// behavior we want.
|
||||
if (!lock) return 'busy' as const
|
||||
// Re-checked here because the grant is a turn of the event loop away
|
||||
// from the check above, and the driver's turn-end is both what frees
|
||||
// the lock and what lands the status making this tab a watcher — so
|
||||
// the one grant we must refuse is the one that arrives this way.
|
||||
if (runHeldElsewhere(sessionId)) return 'busy' as const
|
||||
entered = true
|
||||
return await drive(sessionId, body)
|
||||
}
|
||||
|
||||
@@ -517,11 +517,20 @@ async function applyRemoteSessionPut(id: string): Promise<void> {
|
||||
const token = ++remoteReadSeq
|
||||
remoteReads.set(id, token)
|
||||
const db = await sessionsDb.whenReady()
|
||||
if (!db) return
|
||||
// Only the newest token is cleared on the way out: an older read that lost the
|
||||
// race must leave the winner's token standing.
|
||||
const releaseToken = () => {
|
||||
if (remoteReads.get(id) === token) remoteReads.delete(id)
|
||||
}
|
||||
if (!db) {
|
||||
releaseToken()
|
||||
return
|
||||
}
|
||||
let row: Session | undefined
|
||||
try {
|
||||
row = await db.get('sessions', id)
|
||||
} catch (e) {
|
||||
releaseToken()
|
||||
console.error('Failed to read a session another tab wrote', e)
|
||||
return
|
||||
}
|
||||
@@ -557,16 +566,18 @@ async function applyRemoteSessionPut(id: string): Promise<void> {
|
||||
* the other tab, a seen-watermark bump included. */
|
||||
export function adoptRemoteRow(held: Session, row: Session): void {
|
||||
const stamped = new Map((held.previewTabs ?? []).map((t) => [t.id, t]))
|
||||
// Keys the row no longer carries are removed, not left standing. This file
|
||||
// clears a field by deleting it — see `applyLifecyclePatch`, and the delete of
|
||||
// `archived` an unarchive performs — and `putSessionRow` stores a snapshot, so
|
||||
// a dropped key is how "this is no longer set" arrives. Assigning over the
|
||||
// held object alone keeps the stale value, and this tab's next write to the
|
||||
// record puts it back into the store, undoing what the other tab did.
|
||||
// A draft inside its debounce window is newer than anything the store can
|
||||
// hold, and the pending flush writes this same object, so taking the row's
|
||||
// older text here would end up persisted over what the user is still typing.
|
||||
const pendingDraft = draftPromptFlushHandles.has(held.id) ? held.draftPrompt : undefined
|
||||
// This file clears a field by deleting it and `putSessionRow` stores a
|
||||
// snapshot, so a key the row lacks is how "no longer set" arrives. Assigning
|
||||
// alone keeps the stale value, which this tab's next write puts back.
|
||||
for (const k of Object.keys(held)) {
|
||||
if (!(k in row)) delete (held as Record<string, unknown>)[k]
|
||||
}
|
||||
Object.assign(held, row)
|
||||
if (pendingDraft !== undefined) held.draftPrompt = pendingDraft
|
||||
if (row.previewTabs) {
|
||||
held.previewTabs = row.previewTabs.map((t) => {
|
||||
const live = stamped.get(t.id)
|
||||
|
||||
@@ -529,9 +529,7 @@ describe('adoptRemoteRow', () => {
|
||||
archived: true,
|
||||
archivedByWorkspace: true
|
||||
} as Session
|
||||
// The other tab unarchived: this file clears a field by deleting it, so the
|
||||
// row simply lacks the key. Assigning over the held object would keep the
|
||||
// stale flag, and this tab's next write would put it back in the store.
|
||||
// The other tab unarchived, so the row simply lacks the key.
|
||||
const row = { id: 's1', name: 's1', createdAt: 1 } as Session
|
||||
|
||||
adoptRemoteRow(held, row)
|
||||
@@ -565,6 +563,23 @@ describe('adoptRemoteRow', () => {
|
||||
expect(held.previewTabs?.[0].friendlyLabel).toBe('My script')
|
||||
expect(held.previewTabs?.[0].friendlyPath).toBe('f/a/b')
|
||||
})
|
||||
|
||||
it('keeps a draft still inside its debounce window', () => {
|
||||
// The pending flush writes this same object, so adopting the row's older
|
||||
// text would persist it over what the user is still typing.
|
||||
vi.useFakeTimers()
|
||||
const held = session({ id: 'draft-race', name: 'draft-race', transient: true })
|
||||
sessionState.sessions.push(held)
|
||||
try {
|
||||
setSessionDraftPrompt('draft-race', 'half-typed')
|
||||
adoptRemoteRow(held, { id: 'draft-race', name: 'draft-race', createdAt: 0 } as Session)
|
||||
expect(held.draftPrompt).toBe('half-typed')
|
||||
} finally {
|
||||
vi.clearAllTimers()
|
||||
vi.useRealTimers()
|
||||
sessionState.sessions = sessionState.sessions.filter((s) => s.id !== 'draft-race')
|
||||
}
|
||||
})
|
||||
})
|
||||
|
||||
describe('findEmptyLandingSession — where an unresolvable session link lands', () => {
|
||||
|
||||
Reference in New Issue
Block a user