fix(desktop): host.composer 'new' fails closed, single-owner draft replies, verbs in sdk/composer.ts

Review findings on #120907:

- MAJOR-1: resolveComposerAddress mapped 'new' to target 'active', so
  insertText/submit/focus('new') landed in whatever composer the bus
  routed to (a tile showing session X) while the docs promised the
  session-less draft. 'new' now resolves to the primary composer only
  while it shows no session ($activeSessionId and $selectedStoredSessionId
  both empty — the same condition under which its getIds() is
  ['__new__']), otherwise to no target and the verbs return false / drop.
- minor-1: an id-addressed draft request was answered by every owning
  surface (primary pane + keep-alive tile of one session both painted).
  The first owner stamps `claimed` on the shared detail; later owners skip.
- minor-2: getDraft's stash fallback keyed on ids[0] (the requested,
  possibly runtime, id); the stash is keyed by the stored id, so a
  runtime-addressed read of an unmounted session came back null. Key on
  the resolved stored id.
- minor-3: JSDoc claimed an absent surface "rejects"; nothing rejects —
  wording now matches the null/false contract.
- shape: the ~130 lines of verb bodies + resolver move out of the
  sdk/index.ts facade into sdk/composer.ts (`composer: composerHost`),
  like the settings/bridge lanes.
- tests: collapse the six "no surface -> null/false" assertions and the two
  self-responding plumbing tests; each behaviour keeps <= 2 tests, the
  three findings above are folded into existing tests (all red on 8fe7b682).
This commit is contained in:
teknium1
2026-09-23 19:49:43 -07:00
committed by Teknium
parent 931619742b
commit ef70b3661c
6 changed files with 240 additions and 198 deletions

View File

@@ -412,12 +412,16 @@ describe('composer draft requests', () => {
expect(await requestComposerGetDraft([], { active: true })).toEqual({ text: 'on screen' })
})
it('writes only the addressed session and reports success', async () => {
it('writes only the addressed session, through exactly one owner, and reports success', async () => {
const a = mountDraft('sess-a', 'x')
// A second owner of the same id (primary pane + keep-alive tile showing
// one session) must not paint too: the first claim wins.
const aTwin = mountDraft('sess-a', 'x')
const b = mountDraft('sess-b', 'y')
expect(await requestComposerSetDraft(['sess-a'], 'new text')).toBe(true)
expect(a.wrote).toBe('new text')
expect(aTwin.wrote).toBeNull()
expect(b.wrote).toBeNull()
})

View File

@@ -414,6 +414,9 @@ export interface DraftRequestDetail {
active?: boolean
/** Set-draft payload; absent on read requests. */
text?: string
/** Stamped by the first owning surface so a second owner of the same id
* (the primary pane plus a keep-alive tile showing that session) skips it. */
claimed?: boolean
}
interface DraftReplyDetail {
@@ -551,11 +554,17 @@ export const onComposerDraftRequests = (
return
}
// `active` requests belong to exactly ONE surface — the composer the focus
// bus routes to. Every mounted surface claiming them (the previous
// Exactly ONE surface answers a request. `active` belongs to the composer
// the focus bus routes to — every mounted surface claiming it (the previous
// behavior) let listener registration order decide instead: with keep-alive
// tabs in the stack a buried composer answered the read, and a `set`
// painted onto every mounted draft.
// painted onto every mounted draft. An id-addressed request can have two
// owners (the primary pane and a keep-alive tile showing the same session);
// the first to see it claims it and the other skips.
if (e.detail.claimed) {
return
}
if (e.detail.active) {
if (!address.isActive()) {
return
@@ -568,6 +577,8 @@ export const onComposerDraftRequests = (
}
}
e.detail.claimed = true
if (e.type === GET_DRAFT_EVENT) {
window.dispatchEvent(
new CustomEvent<DraftReplyDetail>(DRAFT_REPLY_EVENT, {

View File

@@ -0,0 +1,144 @@
import {
type ComposerInsertMode,
type ComposerTarget,
requestComposerFocus,
requestComposerGetDraft,
requestComposerInsertAcked,
requestComposerSetDraft,
requestComposerSubmit
} from '@/app/chat/composer/focus'
import { NEW_SESSION_DRAFT_KEY, takeSessionDraft } from '@/store/composer'
import { $activeSessionId, $selectedStoredSessionId } from '@/store/session'
import { $sessionStates } from '@/store/session-states'
/**
* A plugin's session address resolved for the two composer buses. `target` is
* the focus/insert/submit routing key (`null` = no mounted surface can own the
* address, so those verbs fail closed); `ids` are the session ids a mounted
* surface answers draft read/write requests for, and `stored` the durable id
* the persisted stash is keyed by.
*
* `null`/empty = the active composer (bus-resolved, like the internal helpers).
* `'new'` = the draft with no session yet — the primary composer while it shows
* no session (a tile always has one); it never falls through to `'active'`,
* which could be a tile holding another session. A stored id routes to that
* session's tile when one is open, else the primary composer (which renders
* that session); a runtime id is mapped to its stored id first.
*/
const resolveComposerAddress = (
sessionId: null | string | undefined
): { ids: string[]; stored: string; target: 'active' | ComposerTarget | null } => {
const id = typeof sessionId === 'string' ? sessionId.trim() : ''
if (!id) {
return { ids: [], stored: '', target: 'active' }
}
if (id === 'new') {
const primaryIsNewDraft = !$activeSessionId.get() && !$selectedStoredSessionId.get()
return { ids: [NEW_SESSION_DRAFT_KEY], stored: NEW_SESSION_DRAFT_KEY, target: primaryIsNewDraft ? 'main' : null }
}
const stored = $sessionStates.get()[id]?.storedSessionId ?? id
const ids = stored === id ? [id] : [id, stored]
// The primary composer answers only for the session it is showing; a tile
// answers for its own. Anything else stays `tile:<id>` — an absent tile
// drops the request (fail-closed) rather than leaking it into whatever
// session the primary is displaying.
const shownInPrimary = ids.includes($selectedStoredSessionId.get() ?? '')
return { ids, stored, target: shownInPrimary ? 'main' : (`tile:${stored}` as ComposerTarget) }
}
/** THE composer draft surface (#116305 item 1): read, write, insert, and
* submit a session's input WITHOUT touching app DOM — mounted surfaces
* answer for their own sessions over the app's focus bus, so a plugin
* addressing one session can never reach another's composer. Addressing:
* `null` = the active composer (what the user last clicked into); a
* session id (stored or runtime) = that session's composer, whether it is
* the primary surface or a tile; the literal `'new'` = the fresh draft
* with no session id yet. Every verb fails closed: an address that
* resolves to no live surface returns `null`/`false` (or is dropped, for
* `focus`) — it never broadcasts and never throws. */
export const composerHost = {
/** The live draft text of one composer, or null when nothing holds it.
* Mounted surfaces answer with their in-DOM text (current, incl.
* unsaved keystrokes); an unmounted session falls back to its debounced
* persisted stash; `null` address = the active composer (no fallback —
* nobody on screen is answering, which is reported as null). */
getDraft: async (sessionId: null | string = null): Promise<null | string> => {
const { ids, stored } = resolveComposerAddress(sessionId)
if (!ids.length) {
const live = await requestComposerGetDraft([], { active: true })
return live ? live.text : null
}
const live = await requestComposerGetDraft(ids)
if (live) {
return live.text
}
return takeSessionDraft(stored).text || null
},
/** Replace a composer's whole draft. `@`-ref / `/` tokens in the text
* hydrate into chips exactly like an official paste (the app owns the
* markup). Returns false when no mounted surface answers for the
* address — the draft of an unmounted session is never half-written. */
setDraft: async (sessionId: null | string, text: string): Promise<boolean> => {
if (typeof text !== 'string') {
return false
}
const { ids } = resolveComposerAddress(sessionId)
return requestComposerSetDraft(ids, text, ids.length ? undefined : { active: true })
},
/** Append text to a composer's draft through the app's own insert modes
* ('block' = paragraph at end, 'inline' = same line, 'prefix' = start —
* the slash-command seat). Acknowledged like `setDraft`: resolves true
* when a mounted surface claimed and applied the text, false when the
* text trims to nothing or no surface answers for the address within the
* bus settle window — never a silent no-op. */
insertText: (sessionId: null | string, text: string, opts?: { mode?: ComposerInsertMode }): Promise<boolean> => {
const { target } = resolveComposerAddress(sessionId)
if (target === null) {
return Promise.resolve(false)
}
return requestComposerInsertAcked(text, { mode: opts?.mode ?? 'block', target })
},
/** Send `text` as if the user typed it + pressed Enter, and return
* whether a visible surface claimed it. Same fail-closed contract as
* the internal bus: no exact visible composer for the address → false,
* never a broadcast into whichever pane happens to be mounted. */
submit: (sessionId: null | string, text: string): boolean => {
const { target } = resolveComposerAddress(sessionId)
return target !== null && requestComposerSubmit(text, { target })
},
/** Put the caret in a composer — the app's own focus bus, same address
* resolution as the verbs above. `insertText`/`setDraft` already focus a
* VISIBLE surface they paint; this is the standalone verb for the other
* cases (return the caret after a plugin popover/dialog closes, a
* keybind that "goes to the input") that plugins used to reach with a
* hand-built `hermes:composer-focus` CustomEvent. Fail-closed like the
* rest: an absent tile drops the request instead of focusing whatever
* the primary happens to show. */
focus: (sessionId: null | string = null): void => {
const { target } = resolveComposerAddress(sessionId)
if (target !== null) {
requestComposerFocus(target)
}
}
}

View File

@@ -3,7 +3,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
import { ackComposerInsert } from '@/app/chat/composer/focus'
import { createClientSessionState } from '@/lib/chat-runtime'
import { host } from '@/sdk'
import { setActiveSessionId, setAwaitingResponse, setBusy } from '@/store/session'
import { $selectedStoredSessionId, setActiveSessionId, setAwaitingResponse, setBusy } from '@/store/session'
import { clearAllSessionStates, publishSessionState } from '@/store/session-states'
// The warm path must route through the guarded prewarm resolver, not dial the
@@ -266,83 +266,95 @@ describe('host workspace scope', () => {
})
describe('host.composer draft facade', () => {
it('routes insertText by address: tile for a session, resolved-active for null', async () => {
const seen: { mode: string; target: string }[] = []
afterEach(() => {
setActiveSessionId(null)
$selectedStoredSessionId.set(null)
clearAllSessionStates()
})
const off = (event: Event) => {
it('routes insertText and focus by address: tile for a session, resolved-active for null', async () => {
const seen: string[] = []
const onInsert = (event: Event) => {
const { mode, target, token } = (event as CustomEvent).detail
seen.push({ mode, target })
seen.push(`insert:${mode}:${target}`)
// A mounted surface acknowledges the insert like the real composer does.
ackComposerInsert(token, true)
}
window.addEventListener('hermes:composer-insert', off)
const onFocus = (event: Event) => seen.push(`focus:${(event as CustomEvent<{ target: string }>).detail.target}`)
window.addEventListener('hermes:composer-insert', onInsert)
window.addEventListener('hermes:composer-focus', onFocus)
const [tileOk, activeOk] = await Promise.all([
host.composer.insertText('sess-1', ' snippet ', { mode: 'inline' }),
host.composer.insertText(null, 'to active')
])
window.removeEventListener('hermes:composer-insert', off)
expect(tileOk).toBe(true)
expect(activeOk).toBe(true)
expect(seen).toEqual([
{ mode: 'inline', target: 'tile:sess-1' },
{ mode: 'block', target: 'main' }
])
})
it('reports insertText failure on blank text or when no surface answers', async () => {
await expect(host.composer.insertText('sess-1', ' ')).resolves.toBe(false)
// Nothing listens on the insert bus here → the ack settles false, never
// a silent success the plugin would act on.
await expect(host.composer.insertText(null, 'to nobody')).resolves.toBe(false)
})
it('reads the live draft through a bus reply', async () => {
const reply = (event: Event) => {
const { token } = (event as CustomEvent<{ token: number }>).detail
window.dispatchEvent(new CustomEvent('hermes:composer-draft-reply', { detail: { text: 'live text', token } }))
}
window.addEventListener('hermes:composer-get-draft', reply)
const text = await host.composer.getDraft(null)
window.removeEventListener('hermes:composer-get-draft', reply)
expect(text).toBe('live text')
})
it('falls back to the persisted stash when no surface answers', async () => {
const { stashSessionDraft } = await import('@/store/composer')
stashSessionDraft('sess-stash', 'stashed draft', [])
await expect(host.composer.getDraft('sess-stash')).resolves.toBe('stashed draft')
// Never-stashed + unanswered → null (timeout), never a wrong-session read.
await expect(host.composer.getDraft('sess-never')).resolves.toBeNull()
})
it('reports a setDraft failure when no surface answers', async () => {
await expect(host.composer.setDraft('sess-ghost', 'hello')).resolves.toBe(false)
})
it('routes focus by address on the app focus bus: tile for a session, resolved-active for null', async () => {
const seen: string[] = []
const off = (event: Event) => seen.push((event as CustomEvent<{ target: string }>).detail.target)
window.addEventListener('hermes:composer-focus', off)
host.composer.focus('sess-1')
host.composer.focus(null)
// requestComposerFocus defers a plain focus request one macrotask.
await new Promise(resolve => window.setTimeout(resolve, 0))
window.removeEventListener('hermes:composer-focus', off)
window.removeEventListener('hermes:composer-insert', onInsert)
window.removeEventListener('hermes:composer-focus', onFocus)
expect([tileOk, activeOk]).toEqual([true, true])
// A session id never resolves to the primary unless the primary shows it —
// an absent tile drops the request rather than focusing the wrong pane.
expect(seen).toEqual(['tile:sess-1', 'main'])
// an absent tile drops the request rather than reaching the wrong pane.
expect(seen).toEqual(['insert:inline:tile:sess-1', 'insert:block:main', 'focus:tile:sess-1', 'focus:main'])
})
it("addresses 'new' to the session-less primary composer only, never to the active one", async () => {
const seen: string[] = []
const onInsert = (event: Event) => {
const { target, token } = (event as CustomEvent).detail
seen.push(`insert:${target}`)
ackComposerInsert(token, true)
}
const onFocus = (event: Event) => seen.push(`focus:${(event as CustomEvent<{ target: string }>).detail.target}`)
window.addEventListener('hermes:composer-insert', onInsert)
window.addEventListener('hermes:composer-focus', onFocus)
// The primary shows a session → nothing hosts the new draft; the verbs
// fail closed instead of landing in whatever composer the bus routes to.
setActiveSessionId('rt-1')
$selectedStoredSessionId.set('sess-1')
await expect(host.composer.insertText('new', 'x')).resolves.toBe(false)
expect(host.composer.submit('new', 'x')).toBe(false)
host.composer.focus('new')
await new Promise(resolve => window.setTimeout(resolve, 5))
expect(seen).toEqual([])
// No session in the primary → it IS the new draft.
setActiveSessionId(null)
$selectedStoredSessionId.set(null)
await expect(host.composer.insertText('new', 'x')).resolves.toBe(true)
host.composer.focus('new')
await new Promise(resolve => window.setTimeout(resolve, 5))
window.removeEventListener('hermes:composer-insert', onInsert)
window.removeEventListener('hermes:composer-focus', onFocus)
expect(seen).toEqual(['insert:main', 'focus:main'])
})
it('falls back to the persisted stash, keyed by the stored id, when no surface answers', async () => {
const { stashSessionDraft } = await import('@/store/composer')
stashSessionDraft('sess-stash', 'stashed draft', [])
// The stash is keyed by the durable id; a plugin holding the runtime id
// must still reach it once the session states map runtime → stored.
publishSessionState('rt-stash', createClientSessionState('stored-stash'))
stashSessionDraft('stored-stash', 'runtime-addressed', [])
await expect(host.composer.getDraft('sess-stash')).resolves.toBe('stashed draft')
await expect(host.composer.getDraft('rt-stash')).resolves.toBe('runtime-addressed')
})
})

View File

@@ -22,15 +22,6 @@ import { atom, computed, type ReadableAtom } from 'nanostores'
import type { ReactNode } from 'react'
import { capabilityScoped } from '@/api/client'
import {
type ComposerInsertMode,
type ComposerTarget,
requestComposerFocus,
requestComposerGetDraft,
requestComposerInsertAcked,
requestComposerSetDraft,
requestComposerSubmit
} from '@/app/chat/composer/focus'
import { PRIMARY_SESSION_VIEW } from '@/app/chat/session-view'
import { openSession, type OpenSessionIntent } from '@/app/open-session'
import { syncWorkspaceRoute } from '@/app/routes'
@@ -57,7 +48,6 @@ import { registry } from '@/contrib/registry'
import type { WorkspaceMode } from '@/contrib/types'
import { deleteProfile, getLogs, getStatus, hermesApi, type HermesGateway } from '@/hermes'
import { completeMcpDesktopOAuth } from '@/lib/mcp-dashboard-oauth'
import { NEW_SESSION_DRAFT_KEY, takeSessionDraft } from '@/store/composer'
import {
$gateway,
activeGatewayConnectionId,
@@ -116,48 +106,13 @@ import {
import { runGatewayRestart } from '@/store/system-actions'
import type { PaginatedSessions, UsageStats } from '@/types/hermes'
import { composerHost } from './composer'
import { planPluginOpenSession } from './plugin-open-session-plan'
// -- state: readonly views over the app's live atoms -------------------------
const readonlyAtom = <T>(atomLike: ReadableAtom<T>): ReadableAtom<T> => atomLike
/**
* Resolve a plugin's session address to the composer bus's target + the ids a
* mounted surface answers draft requests for. `null` = the active composer
* (bus-resolved, like the internal helpers). A draft chat with no session id
* yet addresses it with the literal `'new'` — the same key the app stashes its
* fresh draft under. A stored id routes to that session's tile when one is
* open, else the primary composer (which renders that session); runtime ids
* are also accepted and answered by the surface holding them.
*/
const resolveComposerAddress = (
sessionId: null | string | undefined
): { ids: string[]; target: 'active' | ComposerTarget } => {
const id = typeof sessionId === 'string' ? sessionId.trim() : ''
if (!id) {
return { ids: [], target: 'active' }
}
if (id === 'new') {
return { ids: [NEW_SESSION_DRAFT_KEY], target: 'active' }
}
// A runtime id is accepted too: resolve it to the durable id the surfaces
// address (session states map runtime → stored while a session is live).
const stored = $sessionStates.get()[id]?.storedSessionId ?? id
const ids = stored === id ? [id] : [id, stored]
// The primary composer answers only for the session it is showing; a tile
// answers for its own. Anything else stays `tile:<id>` — an absent tile
// drops the request (fail-closed) rather than leaking it into whatever
// session the primary is displaying.
const shownInPrimary = ids.includes($selectedStoredSessionId.get() ?? '')
return { ids, target: shownInPrimary ? 'main' : (`tile:${stored}` as ComposerTarget) }
}
/**
* Turn flag for the FOCUSED chat — same semantics as the statusbar's busy
* pulse. While the focused surface is the primary workspace (or a draft with
@@ -1601,93 +1556,7 @@ export const host = {
* active instance changes on a profile swap. */
getGateway: (): HermesGateway | null => $gateway.get(),
/** THE composer draft surface (#116305 item 1): read, write, insert, and
* submit a session's input WITHOUT touching app DOM — mounted surfaces
* answer for their own sessions over the app's focus bus, so a plugin
* addressing one session can never reach another's composer. Addressing:
* `null` = the active composer (what the user last clicked into); a
* session id (stored or runtime) = that session's composer, whether it is
* the primary surface or a tile; the literal `'new'` = the fresh draft
* with no session id yet. Every method is async-safe: an address that
* resolves to no live surface rejects/falses, it never broadcasts. */
composer: {
/** The live draft text of one composer, or null when nothing holds it.
* Mounted surfaces answer with their in-DOM text (current, incl.
* unsaved keystrokes); an unmounted session falls back to its debounced
* persisted stash; `null` address = the active composer (no fallback —
* nobody on screen is answering, which is reported as null). */
getDraft: async (sessionId: null | string = null): Promise<null | string> => {
const { ids } = resolveComposerAddress(sessionId === null ? undefined : sessionId)
if (!ids.length) {
const live = await requestComposerGetDraft([], { active: true })
return live ? live.text : null
}
const live = await requestComposerGetDraft(ids)
if (live) {
return live.text
}
return takeSessionDraft(ids[0]).text || null
},
/** Replace a composer's whole draft. `@`-ref / `/` tokens in the text
* hydrate into chips exactly like an official paste (the app owns the
* markup). Returns false when no mounted surface answers for the
* address — the draft of an unmounted session is never half-written. */
setDraft: async (sessionId: null | string, text: string): Promise<boolean> => {
if (typeof text !== 'string') {
return false
}
const { ids } = resolveComposerAddress(sessionId === null ? undefined : sessionId)
return requestComposerSetDraft(ids, text, ids.length ? undefined : { active: true })
},
/** Append text to a composer's draft through the app's own insert modes
* ('block' = paragraph at end, 'inline' = same line, 'prefix' = start —
* the slash-command seat). Acknowledged like `setDraft`: resolves true
* when a mounted surface claimed and applied the text, false when the
* text trims to nothing or no surface answers for the address within the
* bus settle window — never a silent no-op. */
insertText: (
sessionId: null | string,
text: string,
opts?: { mode?: ComposerInsertMode }
): Promise<boolean> => {
const { target } = resolveComposerAddress(sessionId === null ? undefined : sessionId)
return requestComposerInsertAcked(text, { mode: opts?.mode ?? 'block', target })
},
/** Send `text` as if the user typed it + pressed Enter, and return
* whether a visible surface claimed it. Same fail-closed contract as
* the internal bus: no exact visible composer for the address → false,
* never a broadcast into whichever pane happens to be mounted. */
submit: (sessionId: null | string, text: string): boolean => {
const { target } = resolveComposerAddress(sessionId === null ? undefined : sessionId)
return requestComposerSubmit(text, { target })
},
/** Put the caret in a composer — the app's own focus bus, same address
* resolution as the verbs above. `insertText`/`setDraft` already focus a
* VISIBLE surface they paint; this is the standalone verb for the other
* cases (return the caret after a plugin popover/dialog closes, a
* keybind that "goes to the input") that plugins used to reach with a
* hand-built `hermes:composer-focus` CustomEvent. Fail-closed like the
* rest: an absent tile drops the request instead of focusing whatever
* the primary happens to show. */
focus: (sessionId: null | string = null): void => {
const { target } = resolveComposerAddress(sessionId === null ? undefined : sessionId)
requestComposerFocus(target)
}
}
composer: composerHost
}
// -- react bridge -------------------------------------------------------------

View File

@@ -470,7 +470,9 @@ host.composer: {
**Arbitration.** Every verb is fail-closed on its address: a request is
answered only by the mounted composer that owns that session (its tile, or the
primary pane when it shows that session); `null` is answered only by the surface
the app's focus bus currently routes to. No exact surface → `null`/`false`, never
the app's focus bus currently routes to; `'new'` only by the primary pane while it
shows no session — it never falls through to the active composer. No exact
surface → `null`/`false`, never
a broadcast into whichever pane happens to be mounted. Writes go through the
app's own paint path, so `@`-ref / `/`-command tokens hydrate as chips and the
result is byte-for-byte what the user would get by pasting. These are discrete,