mirror of
https://github.com/windmill-labs/windmill.git
synced 2026-09-21 00:02:30 +00:00
fix(ai-sessions): never enter a catch-up no runtime in this tab can finish
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
co-authored by
Claude Opus 5
parent
d1534ee40b
commit
2e83d582a6
@@ -13,7 +13,8 @@ import { runChatLoop } from './chatLoop'
|
||||
import { clearWorkspaceRoleCache } from '$lib/user'
|
||||
import {
|
||||
noteDriverAlive,
|
||||
noteRemoteTurnEnded
|
||||
noteRemoteTurnEnded,
|
||||
onDriverLost
|
||||
} from '$lib/components/sessions/sessionRunOwner.svelte'
|
||||
|
||||
// This suite forces esm-env BROWSER=true (below). That makes @sveltejs/kit's
|
||||
@@ -332,6 +333,9 @@ describe('AIChatManager cross-tab run guard', () => {
|
||||
const manager = new AIChatManager()
|
||||
manager.isSessionChat = true
|
||||
manager.sessionId = 'session-catching-up'
|
||||
// A tab showing a session has sessionRuntime loaded, which is what makes
|
||||
// `catchingUp` a position it can actually leave.
|
||||
onDriverLost(() => {})
|
||||
noteDriverAlive('session-catching-up', false)
|
||||
noteRemoteTurnEnded('session-catching-up')
|
||||
const restoreInstructions = vi.fn(() => true)
|
||||
|
||||
@@ -84,10 +84,21 @@ export function noteDriverAlive(sessionId: string, planMode: boolean): void {
|
||||
}
|
||||
|
||||
/** The driver says its turn is over. The re-read that follows is what actually
|
||||
* frees this tab, so the position moves to `catchingUp` rather than to idle. */
|
||||
* frees this tab, so the position moves to `catchingUp` rather than to idle —
|
||||
* but only where something can perform that re-read.
|
||||
*
|
||||
* The channel calls this from module scope in every tab that has it, including
|
||||
* ones that never load sessionRuntime (a page carrying the session sidebar with
|
||||
* no session open). Only sessionRuntime can call {@link noteCaughtUp}, so
|
||||
* entering `catchingUp` there would be entering a state with nothing able to
|
||||
* leave it — and `mirroringRemoteRun` reads true for as long as it lasts, which
|
||||
* locks the composer against a run that has already ended and drops the edits
|
||||
* mask and background-job writes that expect the re-read to reseed them. With
|
||||
* no runtime there is no mirrored transcript to be out of step with, so idle is
|
||||
* the truthful position rather than merely the convenient one. */
|
||||
export function noteRemoteTurnEnded(sessionId: string): void {
|
||||
if (runPosition(sessionId).state !== 'watching') return
|
||||
positions.set(sessionId, { state: 'catchingUp' })
|
||||
positions.set(sessionId, canCompleteCatchUp() ? { state: 'catchingUp' } : { state: 'idle' })
|
||||
}
|
||||
|
||||
/** The re-read finished: this tab's transcript and history are one conversation
|
||||
@@ -214,6 +225,13 @@ export function onDriverLost(fn: (sessionId: string) => void): void {
|
||||
driverLost = fn
|
||||
}
|
||||
|
||||
/** Whether a module that can finish a catch-up is loaded in this tab. It is the
|
||||
* same registration either way: sessionRuntime owns both the re-read and the
|
||||
* handler, so having one means having the other. */
|
||||
function canCompleteCatchUp(): boolean {
|
||||
return driverLost !== undefined
|
||||
}
|
||||
|
||||
// Runs only while some session is being driven elsewhere, and stops itself once
|
||||
// none is — a browser with a single tab open never arms it at all.
|
||||
let reaperTimer: ReturnType<typeof setInterval> | undefined
|
||||
|
||||
@@ -3,6 +3,7 @@ import {
|
||||
clearRunPosition,
|
||||
noteDriverAlive,
|
||||
noteRemoteTurnEnded,
|
||||
onDriverLost,
|
||||
withSessionRunLock
|
||||
} from './sessionRunOwner.svelte'
|
||||
|
||||
@@ -28,6 +29,7 @@ describe('withSessionRunLock with no lock to take', () => {
|
||||
// go — but the transcript on screen is still paired with the history from
|
||||
// before that turn, and sending would put that pair to the model.
|
||||
it('refuses while still catching up on a finished turn', async () => {
|
||||
onDriverLost(() => {})
|
||||
noteDriverAlive('session-catching-up', false)
|
||||
noteRemoteTurnEnded('session-catching-up')
|
||||
const body = vi.fn(async () => 'ran')
|
||||
@@ -43,3 +45,25 @@ describe('withSessionRunLock with no lock to take', () => {
|
||||
expect(body).toHaveBeenCalledTimes(1)
|
||||
})
|
||||
})
|
||||
|
||||
// The channel runs these transitions in every tab that loads it, including ones
|
||||
// that never load sessionRuntime — a page carrying the session sidebar with no
|
||||
// session open. Only sessionRuntime performs the re-read that leaves
|
||||
// `catchingUp`, so parking there would strand the session: `mirroringRemoteRun`
|
||||
// stays true, which locks the composer against a run that already ended and
|
||||
// drops the edits-mask and background-job writes that expect the re-read to
|
||||
// reseed them. A fresh module instance is the only honest way to test it — the
|
||||
// registration is process-wide and one-way, exactly as it is in a real tab.
|
||||
describe('a turn ending in a tab with no session runtime', () => {
|
||||
it('settles to idle rather than waiting for a re-read nobody can do', async () => {
|
||||
vi.resetModules()
|
||||
const owner = await import('./sessionRunOwner.svelte')
|
||||
|
||||
owner.noteDriverAlive('session-no-runtime', false)
|
||||
owner.noteRemoteTurnEnded('session-no-runtime')
|
||||
|
||||
expect(owner.isCatchingUp('session-no-runtime')).toBe(false)
|
||||
expect(owner.isMirroring('session-no-runtime')).toBe(false)
|
||||
owner.clearRunPosition('session-no-runtime')
|
||||
})
|
||||
})
|
||||
|
||||
Reference in New Issue
Block a user