diff --git a/packages/ui/src/lib/btw.test.ts b/packages/ui/src/lib/btw.test.ts index 8426b506..32bd27a3 100644 --- a/packages/ui/src/lib/btw.test.ts +++ b/packages/ui/src/lib/btw.test.ts @@ -16,6 +16,7 @@ const upsertedSessions: unknown[] = []; const childStoreSessions: Session[] = []; const currentSessionSwitches: string[] = []; const metadataPatches: Array<{ sessionId: string; result: Record }> = []; +const parentSyncMessages: Message[] = []; mock.module('@/lib/opencode/client', () => ({ opencodeClient: { @@ -48,6 +49,7 @@ mock.module('@/stores/useGlobalSessionsStore', () => ({ })); mock.module('@/sync/sync-refs', () => ({ registerSessionDirectory: (sessionId: string, directory: string) => { registeredDirectories.push(`${sessionId}:${directory}`); }, + getSyncMessages: () => parentSyncMessages, getSyncChildStores: () => ({ children: new Map([['/project', { getState: () => ({ session: childStoreSessions }), @@ -56,7 +58,7 @@ mock.module('@/sync/sync-refs', () => ({ }), })); -const { btwSessionTitle, startBtwSession, destroyBtwSession, promoteBtwSession, filterBtwTailMessages, BTW_BOUNDARY_INSTRUCTION } = +const { btwSessionTitle, startBtwSession, destroyBtwSession, promoteBtwSession, filterBtwTailMessages, findLastCompletedAssistantMessageID, BTW_BOUNDARY_INSTRUCTION } = await import('@/lib/btw'); const { useBtwStore } = await import('@/stores/useBtwStore'); @@ -74,6 +76,15 @@ const record = (id: string): { info: Message; parts: Part[] } => ({ parts: [], }); +// SAFETY: `findLastCompletedAssistantMessageID` reads only `id`, `role` and +// `time`, which are the fields spelled out here. +const assistantMessage = (id: string, completed?: number) => + ({ id, sessionID: 'parent-1', role: 'assistant', time: { created: 1, completed } }) as Message; + +// SAFETY: same narrow read as `assistantMessage`. +const userMessage = (id: string) => + ({ id, sessionID: 'parent-1', role: 'user', time: { created: 1 } }) as Message; + const startInput = { parentSessionId: 'parent-1', question: 'wtf is kafka', @@ -90,6 +101,7 @@ beforeEach(() => { childStoreSessions.length = 0; currentSessionSwitches.length = 0; metadataPatches.length = 0; + parentSyncMessages.length = 0; useBtwStore.setState({ byParent: {} }); forkSessionImpl = () => Promise.reject(new Error('no forkSession stub')); getSessionMessagesImpl = () => Promise.resolve([record('msg-boundary')]); @@ -121,6 +133,17 @@ describe('filterBtwTailMessages', () => { }); }); +describe('findLastCompletedAssistantMessageID', () => { + test('skips an assistant turn that is still streaming', () => { + const messages = [assistantMessage('msg-1', 10), userMessage('msg-2'), assistantMessage('msg-3')]; + expect(findLastCompletedAssistantMessageID(messages)).toBe('msg-1'); + }); + + test('a session with no completed assistant turn has no fork point', () => { + expect(findLastCompletedAssistantMessageID([userMessage('msg-1')])).toBe(null); + }); +}); + describe('startBtwSession', () => { test('forks, marks the fork, links the parent, and routes the question to the fork', async () => { forkSessionImpl = (sessionId, messageId, directory) => { @@ -151,6 +174,32 @@ describe('startBtwSession', () => { expect(useBtwStore.getState().byParent).toEqual({}); }); + test('forks at the last completed assistant turn, not at the in-flight one', async () => { + parentSyncMessages.push(assistantMessage('msg-1', 10), userMessage('msg-2'), assistantMessage('msg-3')); + const forkPoints: Array = []; + forkSessionImpl = (_sessionId, messageId) => { + forkPoints.push(messageId); + return Promise.resolve(makeSession('fork-1', '/project')); + }; + + await startBtwSession(startInput); + + expect(forkPoints).toEqual(['msg-1']); + }); + + test('the boundary falls back to the fork point when the cloned tail reads empty', async () => { + parentSyncMessages.push(assistantMessage('msg-1', 10)); + forkSessionImpl = () => Promise.resolve(makeSession('fork-1', '/project')); + getSessionMessagesImpl = () => Promise.resolve([]); + + await startBtwSession(startInput); + + // Not `null`: a null boundary would show the whole inherited transcript. + expect(metadataPatches[0]?.result).toEqual({ + openchamber: { kind: 'btw', originalSessionID: 'parent-1', btwBoundaryMessageID: 'msg-1' }, + }); + }); + test('the first question carries the boundary instruction as a synthetic part', async () => { forkSessionImpl = () => Promise.resolve(makeSession('fork-1', '/project')); const sentParts: unknown[] = []; diff --git a/packages/ui/src/lib/btw.ts b/packages/ui/src/lib/btw.ts index c19761df..4e24cbe7 100644 --- a/packages/ui/src/lib/btw.ts +++ b/packages/ui/src/lib/btw.ts @@ -4,7 +4,7 @@ import * as sessionActions from '@/sync/session-actions'; import { withBtwSessionLink, withBtwSessionMarker, withoutBtwSessionLink, withoutBtwSessionMarker } from '@/lib/sessionBtwMetadata'; import { useBtwStore } from '@/stores/useBtwStore'; import { useSessionUIStore } from '@/sync/session-ui-store'; -import { getSyncChildStores, registerSessionDirectory } from '@/sync/sync-refs'; +import { getSyncChildStores, getSyncMessages, registerSessionDirectory } from '@/sync/sync-refs'; import { Binary } from '@/sync/binary'; /** @@ -56,9 +56,31 @@ export const BTW_BOUNDARY_INSTRUCTION = [ ].join('\n'); /** The boundary as an `additionalParts` entry for `sendMessage`. */ -export const btwBoundaryParts = (): Array<{ text: string; synthetic: true }> => +const btwBoundaryParts = (): Array<{ text: string; synthetic: true }> => [{ text: BTW_BOUNDARY_INSTRUCTION, synthetic: true }]; +/** + * The parent's last assistant turn that actually finished. + * + * `/btw` is typically typed *while* the main thread is working — that is the + * moment a side question comes up. Forking at HEAD then clones a turn that is + * still streaming: the fork inherits a truncated assistant message and the + * user instruction that provoked it as the newest, most salient thing in its + * context. Anchoring the fork to the last completed turn instead means the + * inherited transcript is always a settled conversation. + * + * Returns `null` when the parent has no completed assistant turn yet (a brand + * new session); the caller then keeps the previous fork-at-HEAD behavior. + */ +export const findLastCompletedAssistantMessageID = (messages: readonly Message[]): string | null => { + for (let index = messages.length - 1; index >= 0; index -= 1) { + const message = messages[index]; + if (message?.role !== 'assistant') continue; + if (message.time.completed !== undefined) return message.id; + } + return null; +}; + export const btwSessionTitle = (question: string): string => `btw: ${question}`; /** @@ -82,7 +104,16 @@ export async function startBtwSession(input: StartBtwInput): Promise { setPanelState(input.parentSessionId, { creating: true }); try { await sessionActions.waitForConnectionOrThrow(); - const forked = await opencodeClient.forkSession(input.parentSessionId, undefined, input.directory); + // Fork at the parent's last completed assistant turn rather than at HEAD, + // so a `/btw` typed mid-turn does not inherit a half-finished one. + const forkPointMessageID = findLastCompletedAssistantMessageID( + getSyncMessages(input.parentSessionId, input.directory), + ); + const forked = await opencodeClient.forkSession( + input.parentSessionId, + forkPointMessageID ?? undefined, + input.directory, + ); // The server may canonicalize the worktree path; the prompt must use the // same directory identity as the forked session. @@ -96,7 +127,14 @@ export async function startBtwSession(input: StartBtwInput): Promise { // id of the newest cloned message. Message ids are server-generated and // ascending, so everything the fork produces sorts after it. const newestCloned = await opencodeClient.getSessionMessages(forked.id, 1, sessionDirectory); - const boundaryMessageID = newestCloned[newestCloned.length - 1]?.info.id ?? null; + // A `null` boundary makes the panel show every inherited message, so an + // empty read must not be taken as "the fork inherited nothing" when we + // know it did: having picked a fork point proves the parent had turns. + // Fall back to that id — the fork's own messages are created later and + // still sort after it, so the tail stays complete either way. + const boundaryMessageID = newestCloned[newestCloned.length - 1]?.info.id + ?? forkPointMessageID + ?? null; // The fork inherits the parent's metadata and title wholesale: replace // the metadata with the btw marker, and rename it (rename is