fix(desktop): scope the queued-drain binding check to fromQueue
The binding check landed unscoped, so it fired for every caller passing a
sessionId/storedSessionId pair — not just queue drains. A slash skill
dispatch into a fresh ⌘T tab passes exactly that shape (sessionId=tab
runtime, storedSessionId=tab stored) with no central binding recorded yet,
so the check nulled the target and the kickoff dropped instead of landing
in the tab.
Only a drain pairs identifiers from two different clocks; every other
explicit-target caller resolves both ids in the same tick and is
authoritative by construction. Gate on fromQueue and add the scoping
invariant as a test.
Also refresh the two drain tests for `queued: true`, which prompt.submit
started sending for queued drains in ab68c5efe after this work branched.
Co-authored-by: theone139344 <theone139344@users.noreply.github.com>
This commit is contained in:
parent
8879b9e9b2
commit
8830f22c9d
|
|
@ -1669,7 +1669,8 @@ describe('usePromptActions submit / queue drain semantics', () => {
|
|||
[]
|
||||
|
||||
const requestGateway = vi.fn(
|
||||
async (method: string) => (method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never
|
||||
async (method: string, _params?: Record<string, unknown>) =>
|
||||
(method === 'session.resume' ? { session_id: 'rt-session-b' } : {}) as never
|
||||
)
|
||||
|
||||
let handle: HarnessHandle | null = null
|
||||
|
|
@ -1696,6 +1697,7 @@ describe('usePromptActions submit / queue drain semantics', () => {
|
|||
expect(requestGateway).toHaveBeenCalledWith(
|
||||
'prompt.submit',
|
||||
{
|
||||
queued: true,
|
||||
session_id: 'rt-session-b',
|
||||
text: 'queued for B mid-switch'
|
||||
},
|
||||
|
|
@ -1717,7 +1719,7 @@ describe('usePromptActions submit / queue drain semantics', () => {
|
|||
// Same window, but B's runtime binding is already known centrally — the
|
||||
// drain should adopt the authoritative binding directly (no resume
|
||||
// round-trip) rather than trusting the leftover foreground id.
|
||||
const requestGateway = vi.fn(async () => ({}) as never)
|
||||
const requestGateway = vi.fn(async (_method: string, _params?: Record<string, unknown>) => ({}) as never)
|
||||
|
||||
let handle: HarnessHandle | null = null
|
||||
render(
|
||||
|
|
@ -1739,6 +1741,7 @@ describe('usePromptActions submit / queue drain semantics', () => {
|
|||
expect(requestGateway).toHaveBeenCalledWith(
|
||||
'prompt.submit',
|
||||
{
|
||||
queued: true,
|
||||
session_id: 'rt-session-b-live',
|
||||
text: 'queued for B, B already re-bound'
|
||||
},
|
||||
|
|
@ -1753,6 +1756,41 @@ describe('usePromptActions submit / queue drain semantics', () => {
|
|||
).toBe(true)
|
||||
})
|
||||
|
||||
it('a NON-queue explicit target keeps its runtime id even with no central binding recorded', async () => {
|
||||
// The scoping invariant for the check above. A slash skill dispatch into a
|
||||
// fresh ⌘T tab passes the same shape a stale drain does — sessionId and
|
||||
// storedSessionId differ, and the tab has no central binding yet — but its
|
||||
// two ids were resolved in the same tick, so the explicit target IS
|
||||
// authoritative. Validating this caller against the (empty) binding would
|
||||
// null the target and silently drop the kickoff into nowhere.
|
||||
const requestGateway = vi.fn(async () => ({}) as never)
|
||||
|
||||
let handle: HarnessHandle | null = null
|
||||
render(
|
||||
<Harness
|
||||
getRuntimeIdForStoredSession={() => null}
|
||||
onReady={h => (handle = h)}
|
||||
refreshSessions={async () => undefined}
|
||||
requestGateway={requestGateway}
|
||||
/>
|
||||
)
|
||||
|
||||
const accepted = await handle!.submitText('kickoff for the tab', {
|
||||
sessionId: 'rt-tab',
|
||||
storedSessionId: 'stored-tab'
|
||||
})
|
||||
|
||||
expect(accepted).toBe(true)
|
||||
expect(requestGateway).toHaveBeenCalledWith(
|
||||
'prompt.submit',
|
||||
{
|
||||
session_id: 'rt-tab',
|
||||
text: 'kickoff for the tab'
|
||||
},
|
||||
1_800_000
|
||||
)
|
||||
})
|
||||
|
||||
it('a fromQueue drain with null runtime id does NOT land in the foreground session (cross-session leak guard)', async () => {
|
||||
// The cross-session leak: a background drain fires with sessionId=null
|
||||
// (the stored session's runtime was reaped by the gateway). Without the
|
||||
|
|
|
|||
|
|
@ -198,21 +198,31 @@ export function useSubmitPrompt(deps: SubmitPromptDeps) {
|
|||
|
||||
let sessionId: null | string = options?.sessionId ?? (isBackgroundQueueDrain ? null : activeSessionIdRef.current)
|
||||
|
||||
// An explicit queued runtime id is authoritative ONLY while it still
|
||||
// belongs to its stored session. On a session switch the composer's
|
||||
// queue key flips with the route while the foreground runtime id lags a
|
||||
// resume behind, so a drain can fire with storedSessionId=B but
|
||||
// sessionId=A-runtime — and the prompt.submit below would land B's
|
||||
// queued prompt (and its whole answer turn) inside A. Verify the pair
|
||||
// against the central binding and drop a stale explicit id: the
|
||||
// targetStoredSessionId resume path below then rebinds the right
|
||||
// runtime, exactly as a background drain with an unknown binding does.
|
||||
// A QUEUED runtime id is authoritative ONLY while it still belongs to its
|
||||
// stored session. On a session switch the composer's queue key flips with
|
||||
// the route while the foreground runtime id lags a resume behind, so a
|
||||
// drain can fire with storedSessionId=B but sessionId=A-runtime — and the
|
||||
// prompt.submit below would land B's queued prompt (and its whole answer
|
||||
// turn) inside A. Verify the pair against the central binding and drop a
|
||||
// stale queued id: the targetStoredSessionId resume path below then
|
||||
// rebinds the right runtime, exactly as a background drain with an
|
||||
// unknown binding does.
|
||||
//
|
||||
// Scoped to fromQueue on purpose. Only a drain pairs identifiers from two
|
||||
// different clocks; every other explicit-target caller resolves both ids
|
||||
// in the same tick and is authoritative by construction. A slash skill
|
||||
// dispatch into a fresh ⌘T tab (slash.ts) passes exactly this shape —
|
||||
// sessionId=tab-runtime, storedSessionId=tab-stored, no central binding
|
||||
// recorded yet — so an unscoped check would null the target and silently
|
||||
// drop the kickoff.
|
||||
//
|
||||
// The identity pair (storedSessionId === sessionId) is the fresh-chat
|
||||
// fallback — an unpersisted conversation's queue key IS its runtime id,
|
||||
// so it has no central binding to check against and is left untouched.
|
||||
if (
|
||||
options?.sessionId &&
|
||||
options?.storedSessionId &&
|
||||
options?.fromQueue &&
|
||||
options.sessionId &&
|
||||
options.storedSessionId &&
|
||||
options.storedSessionId !== options.sessionId
|
||||
) {
|
||||
const boundRuntimeId = getRuntimeIdForStoredSession(options.storedSessionId)
|
||||
|
|
|
|||
Loading…
Reference in New Issue