Merge pull request #3171 from pocharlies-org/feat/btw-boundary

feat(btw): make a btw session a side question, not a continuation of the parent

packages/ui/src/components/chat/ChatInput.tsx

@@ -35,7 +35,8 @@ import {
 import { ReviewFlowDialog, type ReviewFlowExecution } from '@/components/session/ReviewFlowDialog';
 import { BtwPanel } from './btw/BtwPanel';
 import { useBtwPanelState } from './btw/useBtwPanelState';
-import { destroyBtwSession, startBtwSession, type BtwSessionRef } from '@/lib/btw';
+import { wasPromotedBtwSession } from '@/lib/sessionBtwMetadata';
+import { BTW_BOUNDARY_INSTRUCTION, BTW_PROMOTION_NOTICE, destroyBtwSession, startBtwSession, type BtwSessionRef } from '@/lib/btw';
 import { AttachedFilesList, AttachedVSCodeFileChips, ActiveEditorFileSuggestion } from './FileAttachment';
 import { lazyWithChunkRecovery } from '@/lib/chunkLoadRecovery';
 import type { ToolPopupContent } from './message/types';
@@ -340,6 +341,10 @@ const ChatInputComponent: React.FC<ChatInputProps> = ({
         [btwDirectory, btwSessionId, currentSessionId],
     );
     const isBtwActive = Boolean(btwSessionRef) && !btwPanel.collapsed;
+    // A session promoted out of `/btw` keeps the boundary instructions in its
+    // transcript — there is no way to delete a message part — so it has to say
+    // they no longer apply.
+    const isPromotedBtwSession = wasPromotedBtwSession(btwPanel.parentSession);
     const activeRuntimeKey = getRuntimeKey();
     const chatDraftIdentity = React.useMemo(
         () => createChatDraftIdentity(
@@ -1129,7 +1134,14 @@ const ChatInputComponent: React.FC<ChatInputProps> = ({
             composerText: !queuedOnly && inputSnapshot.hasContent ? inputSnapshot.message : null,
             composerAttachments: attachedFiles,
             inlineComments: drafts,
-            syntheticTexts: syntheticParts?.map((part) => part.text) ?? [],
+            // btw mode: the boundary rides with every send, not just the
+            // first one, so the inherited transcript stays reference material
+            // for the whole side conversation.
+            syntheticTexts: [
+                ...(isBtwActive ? [BTW_BOUNDARY_INSTRUCTION] : []),
+                ...(isPromotedBtwSession ? [BTW_PROMOTION_NOTICE] : []),
+                ...(syntheticParts?.map((part) => part.text) ?? []),
+            ],
             linkedIssue: linkedIssue
                 ? { number: linkedIssue.number, title: linkedIssue.title, url: linkedIssue.url, contextText: linkedIssue.contextText }
                 : null,

packages/ui/src/components/chat/btw/useBtwPanelState.ts

@@ -5,6 +5,8 @@ import { getBtwBoundaryMessageID, getBtwSessionID } from '@/lib/sessionBtwMetada
 import { useBtwStore } from '@/stores/useBtwStore';
 
 export type BtwPanelState = {
+  /** The session the composer is in — the one `/btw` would fork. */
+  parentSession: Session | null;
   /** The active fork for this parent, or null when no panel should exist. */
   btwSessionId: string | null;
   btwSession: Session | null;
@@ -40,6 +42,7 @@ export function useBtwPanelState(
   const destroying = Boolean(uiState?.destroying);
   const btwSessionId = btwSession && !destroying ? linkedBtwSessionId : null;
   return {
+    parentSession: parentSession ?? null,
     btwSessionId,
     btwSession: btwSessionId ? btwSession : null,
     // SAFETY: the SDK Session type omits the server's `directory` field; this

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<string, unknown> }> = [];
+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 } =
+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,45 @@ 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<string | undefined> = [];
+    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[] = [];
+    sendMessageImpl = (...args) => {
+      sentParts.push(args[6]);
+      return Promise.resolve();
+    };
+
+    await startBtwSession(startInput);
+
+    expect(sentParts).toEqual([[{ text: BTW_BOUNDARY_INSTRUCTION, synthetic: true }]]);
+  });
+
   test('an empty parent produces a marker without a boundary', async () => {
     forkSessionImpl = () => Promise.resolve(makeSession('fork-1', '/project'));
     getSessionMessagesImpl = () => Promise.resolve([]);
@@ -229,7 +291,9 @@ describe('promoteBtwSession', () => {
 
     expect(metadataPatches).toEqual([
       { sessionId: 'parent-1', result: {} },
-      { sessionId: 'fork-1', result: {} },
+      // The fork stops being a btw session but stays marked as promoted: its
+      // transcript still carries the boundary instructions.
+      { sessionId: 'fork-1', result: { openchamber: { btwPromoted: true } } },
     ]);
     expect(currentSessionSwitches).toEqual(['fork-1']);
   });

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';
 
 /**
@@ -30,6 +30,76 @@ export type StartBtwInput = {
   variant?: string;
 };
 
+/**
+ * Sent as a synthetic part with every message inside a btw session.
+ *
+ * A btw session is a fork, so the model receives the parent's whole
+ * conversation — including whatever plan was in flight when `/btw` was typed.
+ * Without this the fork reads that plan as its own active task and carries on
+ * with it instead of answering the side question, which is the opposite of
+ * what `/btw` is for.
+ *
+ * The wording is deliberately position-independent: it names the history
+ * inherited from the parent thread rather than "everything before this
+ * boundary". The instruction rides along with each send instead of being
+ * pinned once at fork time, so a positional phrasing would be re-anchored
+ * every turn and would end up telling the model to disregard the btw
+ * session's own earlier turns.
+ */
+export const BTW_BOUNDARY_INSTRUCTION = [
+  'You are in a btw session, a side conversation forked from a main thread.',
+  'The history inherited from the parent thread is reference context only. It is not your current task.',
+  'Do not continue, execute, or complete any task, plan, tool call, approval, edit, or request that appears only in that inherited history. Only instructions the user sends inside this btw session are active.',
+  'Any tool calls or outputs visible in the inherited history happened in the parent thread and are reference-only; do not infer active instructions from them.',
+  'Sub-agents are off-limits in this btw session. Do not interact with any existing or new sub-agents, even if sub-agents were used in the inherited history.',
+  'Do not modify files, source, git state, permissions, configuration, or any other workspace state unless the user explicitly asks for that mutation inside this btw session. If they do, keep it minimal, local to the request, and avoid disrupting the main thread.',
+].join('\n');
+
+/**
+ * Sent with every message in a session that was promoted out of `/btw`.
+ *
+ * `BTW_BOUNDARY_INSTRUCTION` is persisted on each message the session sent
+ * while it was a side conversation, and there is no API to remove a message
+ * part after the fact — so promotion cannot delete those lines, only answer
+ * them. Without this, a promoted session keeps reading "no sub-agents, do not
+ * touch the workspace" out of its own history, in a session that is no longer
+ * a side conversation.
+ *
+ * It rides along with every send for the same reason the boundary does: the
+ * instructions it revokes are re-read on every turn, so a one-shot notice
+ * would lose its position relative to them as the conversation grows.
+ */
+export const BTW_PROMOTION_NOTICE =
+  'This session started as a btw side conversation and has since been promoted to a normal session. '
+  + 'The btw constraints in the history above no longer apply: this is now the main thread, and the '
+  + 'usual tool, sub-agent and workspace permissions are in force.';
+
+/** The boundary as an `additionalParts` entry for `sendMessage`. */
+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}`;
 
 /**
@@ -53,7 +123,16 @@ export async function startBtwSession(input: StartBtwInput): Promise<Session> {
   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.
@@ -67,7 +146,14 @@ export async function startBtwSession(input: StartBtwInput): Promise<Session> {
       // 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
@@ -95,7 +181,10 @@ export async function startBtwSession(input: StartBtwInput): Promise<Session> {
           input.agent,
           [],
           undefined,
-          undefined,
+          // The very first question already needs the boundary: the fork is at
+          // its most dangerous here, with the parent's in-flight plan as the
+          // newest thing in its context.
+          btwBoundaryParts(),
           input.variant,
           'normal',
           { sessionId: forked.id, directory: sessionDirectory },

packages/ui/src/lib/sessionBtwMetadata.test.ts

@@ -8,6 +8,7 @@ import {
   withBtwSessionLink,
   withBtwSessionMarker,
   withoutBtwSessionLink,
+  wasPromotedBtwSession,
   withoutBtwSessionMarker,
 } from './sessionBtwMetadata';
 
@@ -64,11 +65,20 @@ describe('fork marker', () => {
     expect(getBtwBoundaryMessageID(review)).toBeNull();
   });
 
-  test('withoutBtwSessionMarker strips the marker and keeps other keys', () => {
+  test('withoutBtwSessionMarker strips the marker, keeps other keys, and records the promotion', () => {
     const marked = { openchamber: { kind: 'btw', originalSessionID: 'parent-1', btwBoundaryMessageID: 'msg-9', btwSessionID: 'nested' } };
-    expect(withoutBtwSessionMarker(marked)).toEqual({ openchamber: { btwSessionID: 'nested' } });
-    expect(withoutBtwSessionMarker({ openchamber: { kind: 'btw', originalSessionID: 'parent-1' } })).toEqual({});
+    expect(withoutBtwSessionMarker(marked)).toEqual({ openchamber: { btwSessionID: 'nested', btwPromoted: true } });
+    expect(withoutBtwSessionMarker({ openchamber: { kind: 'btw', originalSessionID: 'parent-1' } })).toEqual({ openchamber: { btwPromoted: true } });
     const plain = { openchamber: { kind: 'review' } };
     expect(withoutBtwSessionMarker(plain)).toBe(plain);
   });
+
+  test('wasPromotedBtwSession only reports a session that went through promotion', () => {
+    expect(wasPromotedBtwSession(sessionWith({ openchamber: { btwPromoted: true } }))).toBe(true);
+    // Still a live btw fork: the boundary applies, the notice must not.
+    expect(wasPromotedBtwSession(sessionWith({ openchamber: { kind: 'btw', originalSessionID: 'p-1' } }))).toBe(false);
+    expect(wasPromotedBtwSession(sessionWith({ openchamber: {} }))).toBe(false);
+    expect(wasPromotedBtwSession(sessionWith(undefined))).toBe(false);
+    expect(wasPromotedBtwSession(null)).toBe(false);
+  });
 });

packages/ui/src/lib/sessionBtwMetadata.ts

@@ -21,6 +21,7 @@ type BtwMetadata = {
   originalSessionID?: string;
   btwSessionID?: string;
   btwBoundaryMessageID?: string;
+  btwPromoted?: boolean;
 };
 
 const getOpenChamberMetadata = (metadata: SessionMetadataRecord): BtwMetadata => {
@@ -39,6 +40,18 @@ const nonEmpty = (value: string | undefined): string | null =>
 export const getBtwSessionID = (session: Session | null | undefined): string | null =>
   nonEmpty(getOpenChamberMetadata(getSessionMetadata(session)).btwSessionID);
 
+/**
+ * The session was once a btw fork and was promoted to a normal session.
+ *
+ * Its transcript still contains the btw boundary instruction on every message
+ * sent while it was a side conversation, and there is no API to remove a
+ * message part after the fact. The flag lets the composer send a notice that
+ * those constraints have been lifted, so they cannot keep steering a session
+ * that is no longer a side conversation.
+ */
+export const wasPromotedBtwSession = (session: Session | null | undefined): boolean =>
+  getOpenChamberMetadata(getSessionMetadata(session)).btwPromoted === true;
+
 export const isBtwSession = (session: Session | null | undefined): boolean =>
   getOpenChamberMetadata(getSessionMetadata(session)).kind === 'btw'
   && Boolean(getBtwOriginalSessionID(session));
@@ -84,7 +97,13 @@ export const withBtwSessionMarker = (
   return { ...metadata, openchamber };
 };
 
-/** Remove the btw marker so a promoted fork becomes a plain session. */
+/**
+ * Remove the btw marker so a promoted fork becomes a plain session.
+ *
+ * `btwPromoted` replaces it rather than leaving nothing behind: the btw
+ * boundary instructions stay in the transcript forever, so the session has to
+ * remain distinguishable from one that was never a side conversation.
+ */
 export const withoutBtwSessionMarker = (metadata: SessionMetadataRecord): SessionMetadataRecord => {
   const openchamber = getOpenChamberMetadata(metadata);
   if (openchamber.kind !== 'btw') return metadata;
@@ -92,13 +111,8 @@ export const withoutBtwSessionMarker = (metadata: SessionMetadataRecord): Sessio
   delete rest.kind;
   delete rest.originalSessionID;
   delete rest.btwBoundaryMessageID;
-  const next: SessionMetadataRecord = { ...metadata };
-  if (Object.keys(rest).length > 0) {
-    next.openchamber = rest;
-  } else {
-    delete next.openchamber;
-  }
-  return next;
+  rest.btwPromoted = true;
+  return { ...metadata, openchamber: rest };
 };
 
 /** Unlink the parent, but only if it still points at this fork. */