fix(btw): stop the boundary from outliving the session it applies to
Review catch: `promoteBtwSession` removes the btw metadata, but the boundary instruction is persisted on every message the session sent while it was a side conversation, and there is no API to delete a message part after the fact. A promoted session therefore 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. Promotion cannot delete those lines, so it answers them instead: `withoutBtwSessionMarker` now leaves `openchamber.btwPromoted`, and the composer sends `BTW_PROMOTION_NOTICE` with every message in a session carrying it — stating that the session is now the main thread and the usual tool, sub-agent and workspace permissions are in force. It rides 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. `btwPromoted` is additive and optional. A session that never went through `/btw` never has it, and one promoted before this change simply keeps the old behavior.
This commit is contained in:
@@ -18,7 +18,7 @@ import {
|
||||
import type { AttachedFile } from '@/stores/types/sessionTypes';
|
||||
import * as sessionActions from '@/sync/session-actions';
|
||||
import { buildLinkedIssue } from '@/lib/linkedIssues';
|
||||
import { useUserMessageHistory } from "@/sync/sync-context";
|
||||
import { useSession, useUserMessageHistory } from "@/sync/sync-context";
|
||||
import { getInlineCommentDraftKey, useInlineCommentDraftStore, type InlineCommentDraft, type InlineCommentDraftTarget } from '@/stores/useInlineCommentDraftStore';
|
||||
import { useSnippetsStore } from '@/stores/useSnippetsStore';
|
||||
import { renderMagicPrompt } from '@/lib/magicPrompts';
|
||||
@@ -35,7 +35,8 @@ import {
|
||||
import { ReviewFlowDialog, type ReviewFlowExecution } from '@/components/session/ReviewFlowDialog';
|
||||
import { BtwPanel } from './btw/BtwPanel';
|
||||
import { useBtwPanelState } from './btw/useBtwPanelState';
|
||||
import { BTW_BOUNDARY_INSTRUCTION, 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';
|
||||
@@ -338,6 +339,14 @@ 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 currentSessionRecord = useSession(
|
||||
currentSessionId,
|
||||
currentSessionDirectoryForSync ?? currentDirectory ?? undefined,
|
||||
);
|
||||
const isPromotedBtwSession = wasPromotedBtwSession(currentSessionRecord);
|
||||
const activeRuntimeKey = getRuntimeKey();
|
||||
const chatDraftIdentity = React.useMemo(
|
||||
() => createChatDraftIdentity(
|
||||
@@ -1131,6 +1140,7 @@ const ChatInputComponent: React.FC<ChatInputProps> = ({
|
||||
// for the whole side conversation.
|
||||
syntheticTexts: [
|
||||
...(isBtwActive ? [BTW_BOUNDARY_INSTRUCTION] : []),
|
||||
...(isPromotedBtwSession ? [BTW_PROMOTION_NOTICE] : []),
|
||||
...(syntheticParts?.map((part) => part.text) ?? []),
|
||||
],
|
||||
linkedIssue: linkedIssue
|
||||
|
||||
@@ -291,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']);
|
||||
});
|
||||
|
||||
@@ -55,6 +55,25 @@ export const BTW_BOUNDARY_INSTRUCTION = [
|
||||
'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 }];
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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. */
|
||||
|
||||
Reference in New Issue
Block a user