From f39e33e9b097c875bbb45ca3014865d3a09416bd Mon Sep 17 00:00:00 2001 From: Jack Zhuang <277994282+os-zhuang@users.noreply.github.com> Date: Mon, 13 Jul 2026 20:03:08 +0800 Subject: [PATCH] =?UTF-8?q?fix(console-ai):=20reliable=20ask=E2=86=92build?= =?UTF-8?q?=20handoff=20auto-send=20+=20second-handoff=20re-carry=20(ADR-0?= =?UTF-8?q?057=20P4)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Swallow fix: the handoff seed gated on the async-resolved activeAgent, which can settle after the conversation id mints, so the deferred-send replay ran with an empty pending and never re-fired (empty build conversation on handoff). Seed now gates on the route (agentSegment, synchronous) + bumps a pendingSignal that useDeferredFirstSend lists in its replay deps. - Second-handoff re-carry: useObjectChat re-arms parentConversationId on each falsy→truthy transition of the prop (singleton ask id repeats; the URL-mirror strip→re-supply is the fresh-arrival signal), and the seed re-arms on each new handoffPrompt. Unit-tested (useDeferredFirstSend post-id-seed replay; transport re-carry). Co-Authored-By: Claude Opus 4.8 --- .changeset/adr-0057-p4-handoff-reliability.md | 26 ++++++++++ .../app-shell/src/console/ai/AiChatPage.tsx | 52 ++++++++++++++----- .../__tests__/useDeferredFirstSend.test.tsx | 34 ++++++++++++ .../useObjectChat.handoffContext.test.tsx | 34 ++++++++++++ packages/plugin-chatbot/src/useObjectChat.ts | 22 ++++++-- 5 files changed, 150 insertions(+), 18 deletions(-) create mode 100644 .changeset/adr-0057-p4-handoff-reliability.md diff --git a/.changeset/adr-0057-p4-handoff-reliability.md b/.changeset/adr-0057-p4-handoff-reliability.md new file mode 100644 index 0000000000..a8ad574819 --- /dev/null +++ b/.changeset/adr-0057-p4-handoff-reliability.md @@ -0,0 +1,26 @@ +--- +"@object-ui/plugin-chatbot": patch +"@object-ui/app-shell": patch +--- + +fix(console-ai): reliable ask→build handoff auto-send + second-handoff context re-carry (ADR-0057 P4) + +Two follow-ups to the P4 "Open in Builder →" handoff: + +- **Auto-send swallow.** The handoff's auto-sent first message could be dropped on + a brand-new build conversation: the seed gated on the async-resolved + `activeAgent`, which can settle *after* the conversation id is minted, so the + deferred-send replay ran with an empty pending and never re-fired. The seed now + gates on the **route** (`agentSegment`, synchronous) and bumps a `pendingSignal` + that `useDeferredFirstSend` lists in its replay deps, so the seed always fires — + no more empty build conversation on handoff. + +- **Second-handoff re-carry.** A second "Open in Builder →" into the (singleton) + build conversation now re-carries the latest ask context. The transport re-arms + `parentConversationId` on each falsy→truthy transition of the prop (the ask + thread is a singleton, so the same id repeats — the fresh-arrival signal is the + transition the URL-mirror produces, not a changed value), and the seed re-arms + on each new `handoffPrompt`. + +Unit-tested: deferred-send replays a post-id seed via the signal; the transport +re-carries across a strip→re-supply cycle. diff --git a/packages/app-shell/src/console/ai/AiChatPage.tsx b/packages/app-shell/src/console/ai/AiChatPage.tsx index 29ce387247..b4cda929ec 100644 --- a/packages/app-shell/src/console/ai/AiChatPage.tsx +++ b/packages/app-shell/src/console/ai/AiChatPage.tsx @@ -512,8 +512,16 @@ export function useDeferredFirstSend(opts: { pendingRef: React.MutableRefObject; /** The real send (resetSuppression + sendMessage + onSent). */ doSend: (content: string, files?: File[]) => void; + /** + * Bumped by the PAGE whenever it sets `pendingRef` out of band (the ADR-0057 + * handoff seed). Refs don't trigger effects, so without this the replay only + * re-runs on id / doSend changes — and a seed that lands AFTER the id resolves + * (the async agent-catalog race) would never fire (the swallow). Listing it in + * the replay deps makes the seed reliably replay. + */ + pendingSignal?: number; }): (content: string, files?: File[]) => void { - const { chatApi, conversationId, pendingRef, doSend } = opts; + const { chatApi, conversationId, pendingRef, doSend, pendingSignal } = opts; const apiMode = Boolean(chatApi); // Replay a deferred first message the instant a conversation id exists — in @@ -526,7 +534,7 @@ export function useDeferredFirstSend(opts: { if (!pending) return; pendingRef.current = null; doSend(pending.content, pending.files); - }, [apiMode, conversationId, pendingRef, doSend]); + }, [apiMode, conversationId, pendingRef, doSend, pendingSignal]); return useCallback( (content: string, files?: File[]) => { @@ -836,20 +844,32 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro // remount that the id-resolution triggers; the freshly-mounted pane replays it // via useDeferredFirstSend. See that hook for the full race. const pendingFirstMessageRef = useRef(null); + // Bumped whenever the pending ref is set OUT OF BAND (the handoff seed below). + // ChatPane's deferred-send replay lists this in its deps, so it re-runs and + // fires the seed even when the conversation id was ALREADY minted. Without it a + // seed that lands after the id resolves is never replayed — the swallow (the + // replay otherwise only re-runs on id / doSend changes). + const [pendingFirstMessageSeq, setPendingFirstMessageSeq] = useState(0); + const stashPendingFirstMessage = useCallback((m: PendingFirstMessage) => { + pendingFirstMessageRef.current = m; + setPendingFirstMessageSeq((s) => s + 1); + }, []); // ADR-0057 P4 — seed the handed-off build prompt as this surface's first - // message. Only on the BUILD surface (the handoff target), once, before the - // conversation id is minted; `useDeferredFirstSend` (in ChatPane) replays it - // the moment the id resolves. The URL-mirror then rewrites to - // `/ai/build/:id?package=…`, dropping `?handoffPrompt`, so a reload never - // re-sends it. - const handoffSeededRef = useRef(false); + // message; `useDeferredFirstSend` (in ChatPane) replays it once the id lands. + // Gate on the ROUTE (`agentSegment`), known synchronously — NOT on the + // async-resolved `activeAgent`, which can settle AFTER the id is minted and so + // seed too late for the replay to see it (the swallow race, cloud#817 P4 + // follow-up #1). Re-arm on each new `handoffPrompt` VALUE so a SECOND handoff + // re-seeds into the singleton build conversation (#2). The URL-mirror strips + // `?handoffPrompt` after the send, so a reload never re-fires. + const lastSeededHandoffPromptRef = useRef(null); useEffect(() => { - if (!handoffPrompt || handoffSeededRef.current) return; - if (!activeAgent || agentRouteName(activeAgent) !== 'build') return; - handoffSeededRef.current = true; - pendingFirstMessageRef.current = { content: handoffPrompt }; - }, [handoffPrompt, activeAgent]); + if (!handoffPrompt || agentSegment !== 'build') return; + if (lastSeededHandoffPromptRef.current === handoffPrompt) return; + lastSeededHandoffPromptRef.current = handoffPrompt; + stashPendingFirstMessage({ content: handoffPrompt }); + }, [handoffPrompt, agentSegment, stashPendingFirstMessage]); const handleSent = useCallback( (firstUserMessage?: string) => { @@ -1005,6 +1025,7 @@ export function AiChatPage({ apiBase: apiBaseProp, defaultAgent: defaultAgentPro parentHandoffConversationId={handoffParentConversationId} initialMessages={initialMessages} pendingFirstMessageRef={pendingFirstMessageRef} + pendingFirstMessageSeq={pendingFirstMessageSeq} onSent={handleSent} onShare={() => setShareOpen(true)} onDebug={() => setDebugOpen(true)} @@ -1088,6 +1109,9 @@ interface ChatPaneProps { /** Page-owned stash for a first message sent before the conversation id resolved * (survives this pane's id-keyed remount). See {@link useDeferredFirstSend}. */ pendingFirstMessageRef: React.MutableRefObject; + /** Bumps when the page seeds `pendingFirstMessageRef` out of band (handoff), so + * the deferred-send replay re-runs. See {@link useDeferredFirstSend}. */ + pendingFirstMessageSeq?: number; onSent: (firstUserMessage?: string) => void; onShare: () => void; /** Opens the Build Doctor drawer (build agent only). */ @@ -1110,6 +1134,7 @@ export function ChatPane({ parentHandoffConversationId, initialMessages, pendingFirstMessageRef, + pendingFirstMessageSeq, onSent, onShare, onDebug, @@ -1380,6 +1405,7 @@ export function ChatPane({ conversationId, pendingRef: pendingFirstMessageRef, doSend, + pendingSignal: pendingFirstMessageSeq, }); const headerSlot = ( diff --git a/packages/app-shell/src/console/ai/__tests__/useDeferredFirstSend.test.tsx b/packages/app-shell/src/console/ai/__tests__/useDeferredFirstSend.test.tsx index 83dd77f332..70ac7f01a7 100644 --- a/packages/app-shell/src/console/ai/__tests__/useDeferredFirstSend.test.tsx +++ b/packages/app-shell/src/console/ai/__tests__/useDeferredFirstSend.test.tsx @@ -58,6 +58,40 @@ describe('useDeferredFirstSend', () => { expect(doSend).toHaveBeenCalledTimes(1); }); + // ADR-0057 P4 follow-up #1 (handoff swallow): the handoff seed sets `pendingRef` + // OUT OF BAND, and can land AFTER the conversation id already resolved (the + // async agent-catalog race). Refs don't trigger effects, so the replay would + // never fire — unless a `pendingSignal` bump re-runs it. This locks that in. + it('replays a pending message seeded AFTER the id resolved, driven by pendingSignal', () => { + const pendingRef: React.MutableRefObject = { current: null }; + const doSend = vi.fn(); + const { rerender } = renderHook( + ({ pendingSignal }) => + useDeferredFirstSend({ chatApi: API, conversationId: 'c1', pendingRef, doSend, pendingSignal }), + { initialProps: { pendingSignal: 0 } }, + ); + // Id already present but nothing seeded yet → no send. + expect(doSend).not.toHaveBeenCalled(); + + // Page seeds the handoff prompt out of band (id already resolved) + bumps the + // signal → the replay fires exactly once. + act(() => { + pendingRef.current = { content: 'handoff prompt', files: undefined }; + rerender({ pendingSignal: 1 }); + }); + expect(doSend).toHaveBeenCalledTimes(1); + expect(doSend).toHaveBeenCalledWith('handoff prompt', undefined); + expect(pendingRef.current).toBeNull(); + + // A SECOND handoff (#2) — seed again + bump again → replays again. + act(() => { + pendingRef.current = { content: 'second handoff', files: undefined }; + rerender({ pendingSignal: 2 }); + }); + expect(doSend).toHaveBeenCalledTimes(2); + expect(doSend).toHaveBeenLastCalledWith('second handoff', undefined); + }); + it('sends straight through when a conversation id is already present', () => { const { result, pendingRef, doSend } = setup({ chatApi: API, conversationId: 'c1' }); diff --git a/packages/plugin-chatbot/src/__tests__/useObjectChat.handoffContext.test.tsx b/packages/plugin-chatbot/src/__tests__/useObjectChat.handoffContext.test.tsx index 5406e7b7f7..74331b7482 100644 --- a/packages/plugin-chatbot/src/__tests__/useObjectChat.handoffContext.test.tsx +++ b/packages/plugin-chatbot/src/__tests__/useObjectChat.handoffContext.test.tsx @@ -79,6 +79,40 @@ describe('useObjectChat — ask→build handoff context (cloud#817)', () => { expect(ctx2).toMatchObject({ agentName: 'build', packageId: 'app.crm' }); }); + it('re-carries on a SECOND handoff (falsy→truthy transition re-arms, even same id)', async () => { + // #2: a second "Open in Builder →" resumes the singleton build conversation + // and re-supplies the SAME ask id. The URL-mirror strips the param between + // handoffs (truthy→falsy→truthy), which is the fresh-arrival signal — a + // value-equality check would miss it. + const fetchMock = vi.fn(async () => dataStreamResponse()); + vi.stubGlobal('fetch', fetchMock); + + const { result, rerender } = renderHook( + ({ parent }: { parent?: string }) => + useObjectChat({ + api: API, + conversationId: 'build_1', + parentConversationId: parent, + body: { context: { agentName: 'build' } }, + }), + { initialProps: { parent: 'ask_42' as string | undefined } }, + ); + + // First handoff turn carries it, then it's consumed. + await act(async () => { result.current.sendMessage('first handoff'); }); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(1)); + expect(contextOf(fetchMock, 0).parentConversationId).toBe('ask_42'); + + // URL-mirror strips the param (truthy→falsy). + rerender({ parent: undefined }); + // Second handoff re-supplies the SAME id (falsy→truthy) → re-armed. + rerender({ parent: 'ask_42' }); + + await act(async () => { result.current.sendMessage('second handoff'); }); + await waitFor(() => expect(fetchMock).toHaveBeenCalledTimes(2)); + expect(contextOf(fetchMock, 1).parentConversationId).toBe('ask_42'); + }); + it('sends no parentConversationId when none was handed off', async () => { const fetchMock = vi.fn(async () => dataStreamResponse()); vi.stubGlobal('fetch', fetchMock); diff --git a/packages/plugin-chatbot/src/useObjectChat.ts b/packages/plugin-chatbot/src/useObjectChat.ts index dea9b5df69..65294e51bc 100644 --- a/packages/plugin-chatbot/src/useObjectChat.ts +++ b/packages/plugin-chatbot/src/useObjectChat.ts @@ -338,12 +338,24 @@ export function useObjectChat(options: UseObjectChatOptions = {}): UseObjectChat const modelRef = useRef(model); modelRef.current = model; - // ADR-0057 P4 / cloud#817 — the handed-off `ask` conversation id. Sent as - // `context.parentConversationId` on the FIRST turn only: the backend redeems - // it once (loading that thread as the build turn's context) and the client - // owns history from there, so re-sending it would re-inject the same block. We - // hold it in a ref and clear it after the first send (below, in prepare). + // ADR-0057 P4 / cloud#817 — the handed-off `ask` conversation id, sent as + // `context.parentConversationId` on the handoff turn: armed here, consumed once + // in prepare below, then cleared so normal follow-ups don't re-carry it. The + // backend redeems it into the build turn's context. + // + // Re-arm on every falsy→truthy transition of the prop (not on a new VALUE): a + // SECOND "Open in Builder →" resumes the same singleton build conversation and + // re-supplies the SAME ask id (the ask thread is a singleton too), so the + // fresh-arrival signal is the transition, not a changed value. The URL-mirror + // strips the param after each handoff send, giving the truthy→falsy edge — so + // a later handoff arms again and its latest ask context re-carries (#2). const parentConvRef = useRef(parentConversationId); + const prevParentConvPropRef = useRef(parentConversationId); + useEffect(() => { + const prev = prevParentConvPropRef.current; + prevParentConvPropRef.current = parentConversationId; + if (parentConversationId && !prev) parentConvRef.current = parentConversationId; + }, [parentConversationId]); // Build a transport for API mode that posts to the configured endpoint and // forwards conversation/system/model metadata in the request body.