opentui(harden): fix 3 triaged findings (timer leak, tool-match scope, complete-only)
Subagent hardening pass over boundary/logic/view, findings triaged (most were false positives or app-lifetime-moot). The 3 genuine fixes: - liveGateway.stop() now clears the pending 16ms coalesce timer before client.stop() — a queued flush() could otherwise fire batch()/handlers into a torn-down store after the layer scope releases. - store.findToolPart now scans only the LIVE (last) assistant turn, not every message — a tool.complete pairs with a tool.start in the current turn, so this avoids matching a same-id tool in an older/resumed turn (and is O(parts)). - store message.complete with text but NO prior start/delta now creates the turn (complete-only gateways) instead of dropping the final text; still no empty bubble when there's no text. +2 regression tests. Triaged as NOT-a-bug / accepted-risk (documented so they're not relitigated): @opentui/solid useKeyboard DOES auto-cleanup (onCleanup keyHandler.off, index.js:59); dimensions/scrollAnchor timers are app-lifetime / try-catch-safe; unbounded-growth, duplicate-dedup, and split-frame are theoretical for a trusted local newline-framed subprocess; clipboard spawn timeout + atomic active-session write are minor follow-ups. 93 pass.
This commit is contained in:
parent
2a86f039ea
commit
aa5489e804
@ -113,7 +113,14 @@ function makeLiveGateway(): { service: GatewayServiceShape; stop: () => void } {
|
|||||||
sessionId: () => sessionId
|
sessionId: () => sessionId
|
||||||
}
|
}
|
||||||
|
|
||||||
return { service, stop: () => client.stop() }
|
// Clear a pending coalesce timer on teardown so a queued flush() can't fire
|
||||||
|
// batch()/handlers into a torn-down store after the layer scope releases.
|
||||||
|
const stop = () => {
|
||||||
|
if (timer) clearTimeout(timer)
|
||||||
|
timer = undefined
|
||||||
|
client.stop()
|
||||||
|
}
|
||||||
|
return { service, stop }
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
|
|||||||
@ -323,16 +323,16 @@ export function createSessionStore() {
|
|||||||
return created
|
return created
|
||||||
}
|
}
|
||||||
|
|
||||||
/** Find a tool part by id, scanning recent assistant turns (complete may land late). */
|
/** Find a tool part by id in the CURRENT (last) assistant turn — a tool.complete
|
||||||
|
* always pairs with a tool.start in the live turn, so scoping there avoids
|
||||||
|
* matching a same-id tool in an older/resumed turn (and is O(parts), not O(all)). */
|
||||||
function findToolPart(draft: StoreState, id: string): ToolPartState | undefined {
|
function findToolPart(draft: StoreState, id: string): ToolPartState | undefined {
|
||||||
for (let i = draft.messages.length - 1; i >= 0; i--) {
|
const parts = liveAssistant(draft)?.parts
|
||||||
const parts = draft.messages[i]?.parts
|
if (!parts) return undefined
|
||||||
if (!parts) continue
|
|
||||||
for (let j = parts.length - 1; j >= 0; j--) {
|
for (let j = parts.length - 1; j >= 0; j--) {
|
||||||
const p = parts[j]
|
const p = parts[j]
|
||||||
if (p && p.type === 'tool' && p.id === id) return p
|
if (p && p.type === 'tool' && p.id === id) return p
|
||||||
}
|
}
|
||||||
}
|
|
||||||
return undefined
|
return undefined
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -473,10 +473,12 @@ export function createSessionStore() {
|
|||||||
case 'message.complete':
|
case 'message.complete':
|
||||||
setState(
|
setState(
|
||||||
produce(draft => {
|
produce(draft => {
|
||||||
const live = liveAssistant(draft, true)
|
// complete-only gateways may send `message.complete{text}` with no prior
|
||||||
|
// start/delta → create the turn so the final text isn't dropped.
|
||||||
|
const finalText = event.payload?.text
|
||||||
|
const live = liveAssistant(draft, true) ?? (finalText ? ensureAssistant(draft) : undefined)
|
||||||
if (!live) return
|
if (!live) return
|
||||||
// If no deltas arrived (complete-only gateways), seed the full text once.
|
// If no deltas arrived (complete-only gateways), seed the full text once.
|
||||||
const finalText = event.payload?.text
|
|
||||||
const hasText = (live.parts ?? []).some(p => p.type === 'text' && p.text.length > 0)
|
const hasText = (live.parts ?? []).some(p => p.type === 'text' && p.text.length > 0)
|
||||||
if (finalText && !hasText) appendPart(live, 'text', finalText)
|
if (finalText && !hasText) appendPart(live, 'text', finalText)
|
||||||
live.streaming = false
|
live.streaming = false
|
||||||
|
|||||||
@ -91,6 +91,24 @@ describe('session store — ordered parts (Phase 2b)', () => {
|
|||||||
}
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
|
test('message.complete with text but NO prior start creates the turn (complete-only gateway; no drop)', () => {
|
||||||
|
const store = createSessionStore()
|
||||||
|
store.apply({ type: 'gateway.ready' })
|
||||||
|
// no message.start / no deltas — straight to complete with the full text
|
||||||
|
store.apply({ type: 'message.complete', payload: { text: 'The whole answer.' } })
|
||||||
|
const msg = store.state.messages.at(-1)!
|
||||||
|
expect(msg.role).toBe('assistant')
|
||||||
|
expect(msg.streaming).toBe(false)
|
||||||
|
expect(msg.parts?.some(p => p.type === 'text' && p.text === 'The whole answer.')).toBe(true)
|
||||||
|
})
|
||||||
|
|
||||||
|
test('message.complete with no live turn and no text does NOT create an empty bubble', () => {
|
||||||
|
const store = createSessionStore()
|
||||||
|
store.apply({ type: 'gateway.ready' })
|
||||||
|
store.apply({ type: 'message.complete', payload: {} })
|
||||||
|
expect(store.state.messages.filter(m => m.role === 'assistant')).toHaveLength(0)
|
||||||
|
})
|
||||||
|
|
||||||
test('tool.complete updates the running tool part IN PLACE (not a new row)', () => {
|
test('tool.complete updates the running tool part IN PLACE (not a new row)', () => {
|
||||||
const store = createSessionStore()
|
const store = createSessionStore()
|
||||||
store.apply({ type: 'message.start' })
|
store.apply({ type: 'message.start' })
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user