fix(tui): abort the resume scan with its overlay
Review findings from ds-review-bot: closing the loading picker now aborts the scan through the AbortSignal both query methods accept, a signal-ignoring backend's late settlement is dropped by a staleness check, one catch spans listing and projection so a projection failure closes the overlay instead of stranding the loading placeholder, setCandidates clears a stale still-loading error, and the batch comment no longer overstates the win as scaling with session count.
This commit is contained in:
@@ -222,21 +222,28 @@ export function createResumeController(deps: ResumeControllerDeps): ResumeContro
|
||||
},
|
||||
})
|
||||
resumeOverlay = session
|
||||
// Closing the picker — Escape, supersession, disposal — aborts the scan:
|
||||
// the borrowed-log pass over a large store must not outlive its overlay.
|
||||
const scanAbort = new AbortController()
|
||||
void session.closed.then(() => {
|
||||
scanAbort.abort()
|
||||
/* v8 ignore next -- overlay FIFO closes this session before a replacement can become the tracked resume overlay */
|
||||
if (resumeOverlay === session) resumeOverlay = undefined
|
||||
})
|
||||
deps.requestRender()
|
||||
void listQuery.listSessions().then(async (records) => {
|
||||
if (deps.isDisposed() || scan !== resumeScan) return
|
||||
/** Whether this scan's overlay, session generation, or TUI is gone. */
|
||||
const scanStale = (): boolean =>
|
||||
deps.isDisposed() || scan !== resumeScan || scanAbort.signal.aborted
|
||||
const scanCandidates = async (): Promise<void> => {
|
||||
const records = await listQuery.listSessions(scanAbort.signal)
|
||||
if (scanStale()) return
|
||||
// Every workspace in the store is summarized; the picker owns the
|
||||
// current-workspace/all-workspaces scope split over the whole set.
|
||||
const providers = new Set(ctx.llm.listProviders().map(provider => provider.id))
|
||||
// One bounded batch projection over borrowed logs: unlike a
|
||||
// per-candidate readSession, it lists persistence once and skips
|
||||
// replay validation and log cloning, so opening the selector scales
|
||||
// with session count instead of total log size. A corrupt neighbor
|
||||
// degrades to one disabled row.
|
||||
// replay validation and log cloning, bounding memory by what each
|
||||
// summary retains. A corrupt neighbor degrades to one disabled row.
|
||||
const recordById = new Map(records.map(record => [record.header.id, record]))
|
||||
const listedRecord = (id: SessionId): SessionRecord => {
|
||||
const record = recordById.get(id)
|
||||
@@ -247,18 +254,23 @@ export function createResumeController(deps: ResumeControllerDeps): ResumeContro
|
||||
const results = await listQuery.projectSessions(
|
||||
records.map(record => record.header.id),
|
||||
source => summarize(listedRecord(source.header.id), source, providers),
|
||||
scanAbort.signal,
|
||||
)
|
||||
const candidates = results.map(result => result.status === 'fulfilled'
|
||||
? result.value
|
||||
: unreadableCandidate(listedRecord(result.sessionId), result.reason))
|
||||
candidates.sort((a, b) => b.lastActivityAt - a.lastActivityAt
|
||||
|| a.record.header.id.localeCompare(b.record.header.id))
|
||||
if (deps.isDisposed() || scan !== resumeScan) return
|
||||
if (scanStale()) return
|
||||
scanned = candidates
|
||||
picker?.setCandidates(candidates)
|
||||
deps.requestRender()
|
||||
}, (error: unknown) => {
|
||||
if (deps.isDisposed() || scan !== resumeScan) return
|
||||
}
|
||||
// One catch covers both stages, so a projection failure cannot strand
|
||||
// the overlay on its loading placeholder; an aborted scan's rejection
|
||||
// stays silent because the user already dismissed the picker.
|
||||
void scanCandidates().catch((error: unknown) => {
|
||||
if (scanStale()) return
|
||||
void session.close()
|
||||
deps.appendNotice(`Resume session scan failed: ${errorChain(error)}`, 'error')
|
||||
})
|
||||
|
||||
@@ -574,6 +574,8 @@ export class ResumePicker implements Component, Focusable {
|
||||
setCandidates(candidates: readonly ResumeCandidate[]): void {
|
||||
this.candidates = candidates
|
||||
this.selectedIndex = 0
|
||||
// A still-loading error is false the moment rows exist.
|
||||
this.error = ''
|
||||
this.invalidate()
|
||||
}
|
||||
|
||||
|
||||
@@ -42,7 +42,8 @@ import {
|
||||
type TuiRuntime,
|
||||
} from '../src/index.ts'
|
||||
import { WorkspaceFileSearch } from '../src/chat/file-autocomplete.ts'
|
||||
import { ATTRIBUTE_ROLES, COLOR_ROLES, paletteSpec } from '../src/components/theme.ts'
|
||||
import { ResumePicker } from '../src/components/dialogs.ts'
|
||||
import { ATTRIBUTE_ROLES, COLOR_ROLES, createPalette, paletteSpec } from '../src/components/theme.ts'
|
||||
import {
|
||||
appendAssistant,
|
||||
appendUser,
|
||||
@@ -651,6 +652,96 @@ describe('goodbye message and /resume', () => {
|
||||
expect(result.terminal.stopped).toBeGreaterThan(0)
|
||||
})
|
||||
|
||||
it('clears the still-loading error the moment scanned rows arrive', () => {
|
||||
const picker = new ResumePicker(
|
||||
undefined,
|
||||
10,
|
||||
'/workspace',
|
||||
() => 30,
|
||||
createPalette(false),
|
||||
() => {},
|
||||
() => {},
|
||||
)
|
||||
picker.focused = true
|
||||
picker.handleInput('\r')
|
||||
expect(picker.render(80).join('\n')).toContain('Sessions are still loading.')
|
||||
picker.setCandidates([])
|
||||
const rendered = picker.render(80).join('\n')
|
||||
expect(rendered).not.toContain('Sessions are still loading.')
|
||||
expect(rendered).toContain('No matching sessions.')
|
||||
})
|
||||
|
||||
it('aborts an in-flight scan when the loading picker is dismissed', async () => {
|
||||
const listing = Promise.withResolvers<SessionRecord[]>()
|
||||
let scanSignal: AbortSignal | undefined
|
||||
let projections = 0
|
||||
const result = await setup({
|
||||
async configureContext(ctx) {
|
||||
ctx.provide('tools', { get: () => undefined } as never)
|
||||
ctx.provide('sessionQuery', {
|
||||
listSessions: (signal?: AbortSignal) => { scanSignal = signal; return listing.promise },
|
||||
projectSessions: async () => { projections += 1; return [] },
|
||||
} as never)
|
||||
},
|
||||
})
|
||||
result.terminal.send('/resume')
|
||||
result.terminal.send('\r')
|
||||
await tick()
|
||||
expect(result.terminal.output).toContain('Loading sessions…')
|
||||
result.terminal.send('\u001B')
|
||||
await tick()
|
||||
expect(scanSignal?.aborted).toBe(true)
|
||||
// A signal-ignoring backend can still fulfill after dismissal: the stale
|
||||
// scan must neither project nor report.
|
||||
listing.resolve([])
|
||||
await tick()
|
||||
expect(projections).toBe(0)
|
||||
expect(result.terminal.output).not.toContain('Resume session scan failed')
|
||||
await dispose(result)
|
||||
})
|
||||
|
||||
it('drops a projection that settles after the picker was dismissed', async () => {
|
||||
const projecting = Promise.withResolvers<never[]>()
|
||||
const result = await setup({
|
||||
async configureContext(ctx) {
|
||||
ctx.provide('tools', { get: () => undefined } as never)
|
||||
ctx.provide('sessionQuery', {
|
||||
listSessions: async () => [],
|
||||
projectSessions: () => projecting.promise,
|
||||
} as never)
|
||||
},
|
||||
})
|
||||
result.terminal.send('/resume')
|
||||
result.terminal.send('\r')
|
||||
await tick()
|
||||
result.terminal.send('\u001B')
|
||||
await tick()
|
||||
projecting.resolve([])
|
||||
await tick()
|
||||
expect(result.terminal.output).not.toContain('(0 of 0)')
|
||||
expect(result.terminal.output).not.toContain('Resume session scan failed')
|
||||
await dispose(result)
|
||||
})
|
||||
|
||||
it('closes the loading picker and reports a scan that fails after listing', async () => {
|
||||
const target = header('projection-explodes', 10, '/workspace')
|
||||
const result = await setup({
|
||||
async configureContext(ctx) {
|
||||
ctx.provide('tools', { get: () => undefined } as never)
|
||||
ctx.provide('sessionQuery', {
|
||||
listSessions: () => Promise.resolve([{ header: target, live: false, persisted: true }]),
|
||||
projectSessions: () => Promise.reject(new Error('projection exploded')),
|
||||
} as never)
|
||||
},
|
||||
})
|
||||
result.terminal.send('/resume')
|
||||
result.terminal.send('\r')
|
||||
await tick(); await tick()
|
||||
expect(result.terminal.output).toContain('Resume session scan failed: projection exploded')
|
||||
expect(result.terminal.stopped).toBe(0)
|
||||
await dispose(result)
|
||||
})
|
||||
|
||||
it('opens a loading picker immediately and swaps in the scanned rows', async () => {
|
||||
const target = header('late-listing', 10, '/workspace')
|
||||
const listing = Promise.withResolvers<SessionRecord[]>()
|
||||
|
||||
Reference in New Issue
Block a user