From a12b9be443541b5045c258a216047c2efccf7c94 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Felipe=20Gen=C3=A9?= <55562247+felipegenef@users.noreply.github.com> Date: Sat, 5 Sep 2026 06:28:17 -0300 Subject: [PATCH] feat(chat): Comments and review in VS Code, like the OpenChamber desktop app (#1724) * feat(chat): render code comments as cards instead of fenced text * fix(vscode): route Add Comment to the active session editor panel * fix(chat): persist queued inline comments and tighten file-chip path matching * feat(vscode): comment on code from the editor * fix(chat): keep attached context in the message and broadcast comment removal * fix(vscode): hold every pending comment and gate both entry points on the workspace * fix(vscode): let only the owning surface decide its comment threads * fix(vscode): drop a comment removed while its delivery was still in flight * test(vscode): cover the in-flight comment removal guard * test(vscode): cover comment removal reaching every chat surface * fix(vscode): give up on a comment the chat never confirmed holding * fix(vscode): retract a comment everywhere before reporting it discarded * fix(chat): preserve queued comment cards * fix: preserve inline comment context across send paths * fix(chat): preserve command routing with context * fix(chat): keep unavailable actions on normal send path --------- Co-authored-by: Bohdan Triapitsyn --- packages/ui/src/components/chat/ChatInput.tsx | 202 +++++++---- .../components/chat/CommandAutocomplete.tsx | 2 +- .../components/chat/composer/DOCUMENTATION.md | 10 + .../__tests__/buildOutgoingMessage.test.ts | 4 +- .../submit/__tests__/slashCommands.test.ts | 31 ++ .../chat/composer/submit/slashCommands.ts | 52 ++- .../chat/message/normalizeUserDisplayParts.ts | 33 ++ packages/ui/src/lib/btw.test.ts | 33 ++ packages/ui/src/lib/btw.ts | 13 +- .../ui/src/lib/messages/contextParts.test.ts | 37 ++ packages/ui/src/lib/messages/contextParts.ts | 81 ++++- ...seInlineCommentDraftStore.callerId.test.ts | 67 ++++ .../src/stores/useInlineCommentDraftStore.ts | 16 +- packages/ui/src/sync/DOCUMENTATION.md | 1 + packages/ui/src/sync/session-ui-store.test.js | 130 +++++++ packages/ui/src/sync/session-ui-store.ts | 119 ++++--- packages/vscode/README.md | 1 + packages/vscode/l10n/bundle.l10n.fr.json | 7 + packages/vscode/l10n/bundle.l10n.json | 7 + packages/vscode/package.json | 43 +++ packages/vscode/package.nls.fr.json | 3 + packages/vscode/package.nls.json | 3 + packages/vscode/src/ChatViewProvider.ts | 36 ++ packages/vscode/src/DOCUMENTATION.md | 7 + packages/vscode/src/InlineCommentThreads.ts | 333 ++++++++++++++++++ .../vscode/src/SessionEditorPanelProvider.ts | 158 ++++++++- .../vscode/src/activePanelRouting.test.ts | 39 ++ packages/vscode/src/activePanelRouting.ts | 17 + packages/vscode/src/extension.ts | 104 ++++++ .../vscode/src/inlineCommentSelection.test.ts | 258 ++++++++++++++ packages/vscode/src/inlineCommentSelection.ts | 199 +++++++++++ packages/vscode/webview/api/bridge.ts | 12 + .../webview/inlineCommentRemovals.test.ts | 60 ++++ .../vscode/webview/inlineCommentRemovals.ts | 49 +++ packages/vscode/webview/main.tsx | 156 +++++++- 35 files changed, 2192 insertions(+), 131 deletions(-) create mode 100644 packages/ui/src/stores/useInlineCommentDraftStore.callerId.test.ts create mode 100644 packages/vscode/src/InlineCommentThreads.ts create mode 100644 packages/vscode/src/activePanelRouting.test.ts create mode 100644 packages/vscode/src/activePanelRouting.ts create mode 100644 packages/vscode/src/inlineCommentSelection.test.ts create mode 100644 packages/vscode/src/inlineCommentSelection.ts create mode 100644 packages/vscode/webview/inlineCommentRemovals.test.ts create mode 100644 packages/vscode/webview/inlineCommentRemovals.ts diff --git a/packages/ui/src/components/chat/ChatInput.tsx b/packages/ui/src/components/chat/ChatInput.tsx index 2ded6c5b..13c2b3bd 100644 --- a/packages/ui/src/components/chat/ChatInput.tsx +++ b/packages/ui/src/components/chat/ChatInput.tsx @@ -144,7 +144,7 @@ import { buildCommandVariables, canRunCommand, findMagicPromptCommand, - parseSlashCommand, + planLocalSlashCommand, } from './composer/submit/slashCommands'; import { useAutocompletePosition } from './composer/state/useAutocompletePosition'; import { useMessageHistory } from './composer/state/useMessageHistory'; @@ -1011,6 +1011,13 @@ const ChatInputComponent: React.FC = ({ const handleQueueMessage = React.useCallback(async () => { const inputSnapshot = getCurrentInputSnapshot(); if (!inputSnapshot.hasContent || !currentSessionId || !messageQueueTarget) return; + + // A local command is run, not queued: the queue delivers text to the + // model, and `/compact` or `/btw` mean nothing there. + if (planLocalSlashCommand(inputSnapshot.message, inputMode, hasDrafts, true)) { + void handleSubmitRef.current(); + return; + } const queueRuntimeKey = getRuntimeKey(); const queueTarget = messageQueueTarget; const queueSessionId = currentSessionId; @@ -1119,7 +1126,7 @@ const ChatInputComponent: React.FC = ({ return; } recordLinkedReferences(queueSessionId, queueTarget.directory, linked); - }, [getCurrentInputSnapshot, currentSessionId, messageQueueTarget, attachedFiles, sanitizeAttachmentsForSend, prepareDocumentMentions, extractInlineFileMentions, agents, currentDirectory, consumePendingSyntheticParts, inlineDraftTarget, consumeDrafts, linkedIssue, linkedPr, linkedLinearIssue, scrollToLatest, clearAttachedFiles, isMobile, addToQueue, currentProviderId, currentModelId, currentAgentName, currentVariant, t]); + }, [getCurrentInputSnapshot, currentSessionId, messageQueueTarget, inputMode, hasDrafts, attachedFiles, sanitizeAttachmentsForSend, prepareDocumentMentions, extractInlineFileMentions, agents, currentDirectory, consumePendingSyntheticParts, inlineDraftTarget, consumeDrafts, linkedIssue, linkedPr, linkedLinearIssue, scrollToLatest, clearAttachedFiles, isMobile, addToQueue, currentProviderId, currentModelId, currentAgentName, currentVariant, t]); /** Put the context a queued message was captured with back on the composer chips. */ const restoreQueuedContext = React.useCallback((context: readonly QueuedContextPart[]) => { @@ -1241,6 +1248,51 @@ const ChatInputComponent: React.FC = ({ return; } + // Local slash commands are planned before anything is taken or + // consumed. An action command must leave the queue and the attached + // context where they are; a prompt command must send that context with + // the prompt it produces. A command the composer cannot run here is not + // a local command at all and goes out as typed. + let commandPlan = !queuedOnly && inputSnapshot.hasContent + ? planLocalSlashCommand(inputSnapshot.message, inputMode, hasDrafts, Boolean(currentSessionId)) + : null; + if (commandPlan?.kind === 'prompt') { + const magicCommand = findMagicPromptCommand(commandPlan.command.name); + const commandIsAvailable = commandPlan.command.name === 'btw' + ? Boolean(currentSessionId) + : magicCommand !== null && canRunCommand(magicCommand, { + hasSession: Boolean(currentSessionId), + hasDraft: newSessionDraftOpen, + }); + if (!commandIsAvailable) commandPlan = null; + } + if (commandPlan?.command.name === 'handoff-review' && (isMobile || isVSCodeRuntime())) commandPlan = null; + + // A failed send returns the typed prompt no matter WHY it failed — + // auth, network, server, anything. Losing a long prompt to a toast is + // the one outcome this handler must never produce. The mentions are + // snapshotted here because sending clears them before it can fail. + const confirmedMentionsSnapshot = new Set(confirmedMentionsRef.current); + const restoreComposerText = () => { + if (queuedOnly || !inputSnapshot.message) return; + for (const mention of confirmedMentionsSnapshot) confirmedMentionsRef.current.add(mention); + if (currentChatDraftIdentityRef.current !== chatDraftIdentity) { + // The user switched sessions mid-send: restore into that + // session's persisted draft, not the visible composer. + writeChatDraft(chatDraftIdentity, inputSnapshot.message, confirmedMentionsRef.current); + return; + } + const currentInput = composerRef.current?.getValue() ?? messageRef.current; + if (!currentInput || currentInput === inputSnapshot.message) { + setMessage(inputSnapshot.message); + writeChatDraft(chatDraftIdentity, inputSnapshot.message, confirmedMentionsRef.current); + } else { + // New typing already lives in the composer; the failed prompt + // joins it instead of clobbering either text. + useInputStore.getState().setPendingInputText(inputSnapshot.message, 'append'); + } + }; + // The projection knows the captured send configuration; the full // messages are taken from the queue only once nothing below can still // bail out, so an early return leaves the queue untouched. @@ -1268,7 +1320,7 @@ const ChatInputComponent: React.FC = ({ // queued-message auto-send hook delivers it as the next turn once the // rejected turn winds down and the session returns to idle. This avoids // aborting the turn (which would surface an "aborted" notice). - if (currentSessionId && !queuedOnly && autoReviewRunning && !isBtwActive) { + if (currentSessionId && !queuedOnly && autoReviewRunning && !isBtwActive && !commandPlan) { void handleQueueMessage(); return; } @@ -1276,7 +1328,7 @@ const ChatInputComponent: React.FC = ({ // btw mode: the child fork's blocking prompts are answered inside the // panel; the composer send goes straight to the fork (routeMessage // queues if the fork's own turn is busy). - if (currentSessionId && !queuedOnly && !isBtwActive) { + if (currentSessionId && !queuedOnly && !isBtwActive && !commandPlan) { // Sending is authoritative for blocking prompts: deny pending // permissions and dismiss open questions for the session subtree, // then queue the message once if either was open. The deny/clear @@ -1296,6 +1348,41 @@ const ChatInputComponent: React.FC = ({ } } + // Action commands change session or UI state and send nothing. The + // command text goes; the queue and whatever the composer had attached + // stay exactly where they are. + if (commandPlan?.kind === 'action' && currentSessionId) { + const actionName = commandPlan.command.name; + setMessage(''); + confirmedMentionsRef.current.clear(); + persistDraftImmediately(chatDraftIdentity, ''); + messageHistory.reset(); + setExpandedInput(false); + if (isMobile) composerRef.current?.blur(); + try { + if (actionName === 'undo') { + await useSessionUIStore.getState().handleSlashUndo(currentSessionId); + scrollToBottom?.(); + } else if (actionName === 'redo') { + await useSessionUIStore.getState().handleSlashRedo(currentSessionId); + scrollToBottom?.(); + } else if (actionName === 'timeline') { + setTimelineDialogOpen(true); + } else if (actionName === 'handoff-review') { + setReviewDialogOpen(true); + } else if (actionName === 'compact') { + await sessionActions.waitForConnectionOrThrow(); + const compactDirectory = useSessionUIStore.getState().getDirectoryForSession(currentSessionId) || currentDirectory || undefined; + await opencodeClient.summarizeSession(currentSessionId, providerIdToSend, modelIdToSend, compactDirectory); + } + } catch (error) { + restoreComposerText(); + if (actionName !== 'compact') throw error; + toast.error(getSubmitErrorMessage(error, t('chat.chatInput.toast.compactFailed'))); + } + return; + } + let sendMessageOptions: { target?: NonNullable; sessionId?: string; @@ -1338,7 +1425,7 @@ const ChatInputComponent: React.FC = ({ // skips anything already in flight, and a message already being // delivered stays out of this send so it cannot go out twice. let queuedMessagesToSend: QueuedMessage[] = []; - if (capturedTarget && hasQueuedMessages) { + if (capturedTarget && hasQueuedMessages && !commandPlan) { try { queuedMessagesToSend = await takeForSend(capturedTarget, queuedMessageId); } catch (error) { @@ -1357,6 +1444,27 @@ const ChatInputComponent: React.FC = ({ const drafts: InlineCommentDraft[] = consumedDraftTarget ? consumeDrafts(consumedDraftTarget) : []; + const restoreConsumedDrafts = () => { + if (consumedDraftTarget && drafts.length > 0) { + useInlineCommentDraftStore.getState().restoreDrafts(consumedDraftTarget, drafts); + } + }; + // Everything a prompt command consumed comes back if it fails: the + // attached context, the typed text, and the files. + const restoreConsumedInput = () => { + restoreConsumedDrafts(); + if (syntheticParts?.length) { + const inputState = useInputStore.getState(); + inputState.setPendingSyntheticParts([...syntheticParts, ...(inputState.pendingSyntheticParts ?? [])]); + } + restoreComposerText(); + if (!queuedOnly && attachedFiles.length > 0) { + const inputState = useInputStore.getState(); + const present = new Set(inputState.attachedFiles.map((attachment) => attachment.id)); + const missing = attachedFiles.filter((attachment) => !present.has(attachment.id)); + if (missing.length > 0) inputState.setAttachedFiles([...inputState.attachedFiles, ...missing]); + } + }; const availableSkillNames = new Set( selectSkillsForDirectory(useSkillsStore.getState(), currentDirectory).map((skill) => skill.name), @@ -1417,44 +1525,15 @@ const ChatInputComponent: React.FC = ({ composerRef.current?.blur(); } - // Local slash commands, normal mode only. - const parsedCommand = inputMode === 'normal' ? parseSlashCommand(primaryText) : null; - if (parsedCommand) { - const { name: commandName, argument } = parsedCommand; + // Prompt commands render a visible prompt (or fork a btw question) and + // send it with everything the composer had attached. + if (commandPlan?.kind === 'prompt') { + const { name: commandName, argument } = commandPlan.command; - // Commands that manipulate session state or open UI rather than - // sending a message. - if (commandName === 'undo' && currentSessionId) { - await useSessionUIStore.getState().handleSlashUndo(currentSessionId); - scrollToBottom?.(); - return; - } - if (commandName === 'redo' && currentSessionId) { - await useSessionUIStore.getState().handleSlashRedo(currentSessionId); - scrollToBottom?.(); - return; - } - if (commandName === 'timeline' && currentSessionId) { - setTimelineDialogOpen(true); - return; - } - if (commandName === 'handoff-review' && currentSessionId && !isMobile && !isVSCodeRuntime()) { - setReviewDialogOpen(true); - return; - } - if (commandName === 'compact' && currentSessionId) { - try { - await sessionActions.waitForConnectionOrThrow(); - const compactDirectory = useSessionUIStore.getState().getDirectoryForSession(currentSessionId) || currentDirectory || undefined; - await opencodeClient.summarizeSession(currentSessionId, currentProviderId, currentModelId, compactDirectory); - } catch (error) { - toast.error(getSubmitErrorMessage(error, t('chat.chatInput.toast.compactFailed'))); - } - return; - } if (commandName === 'btw' && currentSessionId) { const question = argument.trim(); if (!question) { + restoreConsumedInput(); toast.error(t('chat.btw.toast.emptyArgument')); return; } @@ -1462,6 +1541,7 @@ const ChatInputComponent: React.FC = ({ || currentDirectory || null; if (!targetDirectory) { + restoreConsumedInput(); toast.error(t('chat.btw.toast.createFailed')); return; } @@ -1479,22 +1559,21 @@ const ChatInputComponent: React.FC = ({ modelID: modelIdToSend, agent: agentNameToSend, variant: variantToSend, + attachments: primaryAttachments, + additionalParts, }); scrollToBottom?.(); } catch (error) { + restoreConsumedInput(); toast.error(getSubmitErrorMessage(error, t('chat.btw.toast.createFailed'))); } return; } // The rest render a visible prompt plus synthetic instructions and - // send them as one message. + // send them as one message, the attached context riding along. const command = findMagicPromptCommand(commandName); - const commandIsAvailable = command !== null && canRunCommand(command, { - hasSession: Boolean(currentSessionId), - hasDraft: newSessionDraftOpen, - }); - if (command && commandIsAvailable) { + if (command) { const variables = buildCommandVariables(command, argument); try { await sessionActions.waitForConnectionOrThrow(); @@ -1505,15 +1584,16 @@ const ChatInputComponent: React.FC = ({ providerIdToSend, modelIdToSend, agentNameToSend, - [], + primaryAttachments, agentMentionName, - [{ text: instructionsText, synthetic: true }], + [...additionalParts, { text: instructionsText, synthetic: true }], variantToSend, inputMode, sendMessageOptions, ); scrollToBottom?.(); } catch (error) { + restoreConsumedInput(); toast.error(getSubmitErrorMessage(error, t(command.errorToastKey))); } return; @@ -1567,12 +1647,6 @@ const ChatInputComponent: React.FC = ({ inputMode, sendMessageOptions, ); - const restoreConsumedDrafts = () => { - if (consumedDraftTarget && drafts.length > 0) { - useInlineCommentDraftStore.getState().restoreDrafts(consumedDraftTarget, drafts); - } - }; - void sendPromise.then(() => { // On a draft there is no session yet in this closure: the send path // creates one and makes it current before resolving, so the id is @@ -1605,27 +1679,7 @@ const ChatInputComponent: React.FC = ({ console.error('Message send failed:', rawMessage || error); restoreConsumedDrafts(); - - // A failed send returns the typed prompt no matter WHY it failed — - // auth, network, server, anything. Losing a long prompt to a toast - // is the one outcome this handler must never produce. - if (inputSnapshot.message) { - if (currentChatDraftIdentityRef.current !== chatDraftIdentity) { - // The user switched sessions mid-send: restore into that - // session's persisted draft, not the visible composer. - writeChatDraft(chatDraftIdentity, inputSnapshot.message, confirmedMentionsRef.current); - } else { - const currentInput = composerRef.current?.getValue() ?? messageRef.current; - if (!currentInput || currentInput === inputSnapshot.message) { - setMessage(inputSnapshot.message); - writeChatDraft(chatDraftIdentity, inputSnapshot.message, confirmedMentionsRef.current); - } else { - // New typing already lives in the composer; the failed - // prompt joins it instead of clobbering either text. - useInputStore.getState().setPendingInputText(inputSnapshot.message, 'append'); - } - } - } + restoreComposerText(); const isSoftNetworkError = normalized.includes('timeout') || diff --git a/packages/ui/src/components/chat/CommandAutocomplete.tsx b/packages/ui/src/components/chat/CommandAutocomplete.tsx index 355eca35..87229683 100644 --- a/packages/ui/src/components/chat/CommandAutocomplete.tsx +++ b/packages/ui/src/components/chat/CommandAutocomplete.tsx @@ -153,10 +153,10 @@ export const CommandAutocomplete = React.forwardRef { expect(context.map((part) => part.kind)).toEqual(['context', 'synthetic', 'context', 'context', 'context', 'instruction']); expect(context[0]?.kind).toBe('context'); expect(context[0]?.text).toContain('Comment on `src/app.ts` lines 3-5 (modified):'); - expect(context[0]?.kind === 'context' ? context[0].metadata : null) - .toEqual({ [CONTEXT_METADATA_KEY]: contextPayloadFromDraft(commentDraft()) }); + expect(context[0]?.kind === 'context' ? context[0].metadata[CONTEXT_METADATA_KEY] : null) + .toEqual(contextPayloadFromDraft(commentDraft())); expect(context[3]).toEqual({ kind: 'context', text: 'pr-diff', diff --git a/packages/ui/src/components/chat/composer/submit/__tests__/slashCommands.test.ts b/packages/ui/src/components/chat/composer/submit/__tests__/slashCommands.test.ts index 07092920..dd7d75ba 100644 --- a/packages/ui/src/components/chat/composer/submit/__tests__/slashCommands.test.ts +++ b/packages/ui/src/components/chat/composer/submit/__tests__/slashCommands.test.ts @@ -6,6 +6,7 @@ import { findMagicPromptCommand, MAGIC_PROMPT_COMMANDS, parseSlashCommand, + planLocalSlashCommand, } from '../slashCommands'; describe('parseSlashCommand', () => { @@ -62,6 +63,36 @@ describe('findMagicPromptCommand', () => { }); }); +describe('planLocalSlashCommand', () => { + test('an action command retains an attached inline comment', () => { + expect(planLocalSlashCommand('/compact', 'normal', true, true)).toEqual({ + command: { name: 'compact', argument: '' }, + kind: 'action', + attachedContext: 'retain', + }); + }); + + test('prompt commands send attached context instead of disabling command parsing', () => { + expect(planLocalSlashCommand('/summary auth', 'normal', true, true)).toEqual({ + command: { name: 'summary', argument: 'auth' }, + kind: 'prompt', + attachedContext: 'send', + }); + expect(planLocalSlashCommand('/btw why?', 'normal', true, true)?.kind).toBe('prompt'); + }); + + test('session actions stay on the normal send path for a new-session draft', () => { + for (const command of ['compact', 'undo', 'redo', 'timeline']) { + expect(planLocalSlashCommand(`/${command}`, 'normal', false, false)).toBeNull(); + } + }); + + test('shell mode and server-owned commands stay outside local planning', () => { + expect(planLocalSlashCommand('/compact', 'shell', true, true)).toBeNull(); + expect(planLocalSlashCommand('/project-command', 'normal', true, true)).toBeNull(); + }); +}); + describe('canRunCommand', () => { const summary = findMagicPromptCommand('summary')!; const explore = findMagicPromptCommand('explore')!; diff --git a/packages/ui/src/components/chat/composer/submit/slashCommands.ts b/packages/ui/src/components/chat/composer/submit/slashCommands.ts index f8467f2d..905e63e0 100644 --- a/packages/ui/src/components/chat/composer/submit/slashCommands.ts +++ b/packages/ui/src/components/chat/composer/submit/slashCommands.ts @@ -136,6 +136,20 @@ export interface ParsedSlashCommand { argument: string; } +export type LocalSlashCommandPlan = { + command: ParsedSlashCommand; + kind: 'action' | 'prompt'; + attachedContext: 'none' | 'retain' | 'send'; +}; + +const LOCAL_ACTION_COMMANDS = new Set([ + 'undo', + 'redo', + 'timeline', + 'handoff-review', + 'compact', +]); + /** * Read the leading slash command out of a message, if there is one. Only the * first word counts as the command; the rest is its argument. @@ -154,6 +168,42 @@ export function parseSlashCommand(text: string): ParsedSlashCommand | null { }; } +/** + * Plan commands owned by the composer before attached context is consumed. + * Action commands leave that context in the composer; prompt commands send it. + * Unknown commands return null so the OpenCode command router remains authoritative. + */ +export function planLocalSlashCommand( + text: string, + inputMode: 'normal' | 'shell' | undefined, + hasAttachedContext: boolean, + hasSession: boolean, +): LocalSlashCommandPlan | null { + if (inputMode !== 'normal') return null; + const command = parseSlashCommand(text); + if (!command) return null; + + if (LOCAL_ACTION_COMMANDS.has(command.name)) { + if (!hasSession) return null; + + return { + command, + kind: 'action', + attachedContext: hasAttachedContext ? 'retain' : 'none', + }; + } + + if (command.name === 'btw' || findMagicPromptCommand(command.name)) { + return { + command, + kind: 'prompt', + attachedContext: hasAttachedContext ? 'send' : 'none', + }; + } + + return null; +} + /** The prompt-pair command for `name`, or null when it is not one. */ export function findMagicPromptCommand(name: string): MagicPromptCommand | null { return COMMANDS_BY_NAME.get(name) ?? null; @@ -173,7 +223,7 @@ export function canRunCommand( export function buildCommandVariables( command: MagicPromptCommand, argument: string, -): { visible: Record; instructions: Record } { +) { const built = command.buildVariables?.(argument) ?? {}; return { visible: built.visible ?? {}, diff --git a/packages/ui/src/components/chat/message/normalizeUserDisplayParts.ts b/packages/ui/src/components/chat/message/normalizeUserDisplayParts.ts index 5d79e703..263e5fc2 100644 --- a/packages/ui/src/components/chat/message/normalizeUserDisplayParts.ts +++ b/packages/ui/src/components/chat/message/normalizeUserDisplayParts.ts @@ -117,10 +117,43 @@ const shouldKeepSyntheticUserText = (text: string, planModeEnabled: boolean): bo return false; }; +const redundantCommentFileUrls = (parts: Part[]): Set => { + const comments = parts + .map((part) => readContextPart(part)) + .filter((payload) => payload?.kind === 'code-comment'); + if (comments.length === 0) return new Set(); + + const redundant = new Set(); + for (const part of parts) { + if (part.type !== 'file') continue; + const { url } = part; + const range = url.match(/[?&]start=(\d+)&end=(\d+)/); + if (!range) continue; + const encodedPath = url.replace(/^file:\/\//, '').split('?')[0]; + let path = encodedPath; + try { + path = decodeURIComponent(encodedPath); + } catch { + // Keep the encoded path; malformed URLs must not break rendering. + } + path = path.replace(/\\/g, '/'); + const matches = comments.some((comment) => { + const commentPath = comment.fileLabel.replace(/\\/g, '/'); + return comment.startLine === Number(range[1]) + && comment.endLine === Number(range[2]) + && (path === commentPath || path.endsWith(`/${commentPath}`)); + }); + if (matches) redundant.add(url); + } + return redundant; +}; + export const normalizeUserDisplayParts = (parts: Part[], options?: { planModeEnabled?: boolean }): Part[] => { const planModeEnabled = options?.planModeEnabled === true; + const redundantFileUrls = redundantCommentFileUrls(parts); return parts .filter((part) => { + if (part.type === 'file' && redundantFileUrls.has(part.url)) return false; const synthetic = (part as { synthetic?: boolean }).synthetic === true; if (!synthetic) return true; if (part.type !== 'text') return false; diff --git a/packages/ui/src/lib/btw.test.ts b/packages/ui/src/lib/btw.test.ts index 4b13285e..4442ac33 100644 --- a/packages/ui/src/lib/btw.test.ts +++ b/packages/ui/src/lib/btw.test.ts @@ -1,5 +1,6 @@ import { beforeEach, describe, expect, mock, test } from 'bun:test'; import type { Message, Part, Session } from '@opencode-ai/sdk/v2'; +import type { StartBtwInput } from './btw'; let forkSessionImpl: (sessionId: string, messageId?: string, directory?: string | null) => Promise; let getSessionMessagesImpl: (id: string, limit?: number, directory?: string | null) => Promise>; @@ -213,6 +214,38 @@ describe('startBtwSession', () => { expect(sentParts).toEqual([[{ text: BTW_BOUNDARY_INSTRUCTION, synthetic: true }]]); }); + test('the first question keeps inline comment context', async () => { + forkSessionImpl = () => Promise.resolve(makeSession('fork-1', '/project')); + const commentPart: NonNullable[number] = { + text: 'Comment on `src/auth.ts` lines 4-4:\n```ts\nauth();\n```\n\ncheck this', + synthetic: true, + metadata: { + openchamberContext: { + kind: 'code-comment', + source: 'file', + fileLabel: 'src/auth.ts', + startLine: 4, + endLine: 4, + language: 'ts', + code: 'auth();', + text: 'check this', + }, + }, + }; + let sentParts: unknown; + sendMessageImpl = (...args) => { + sentParts = args[6]; + return Promise.resolve(); + }; + + await startBtwSession({ ...startInput, additionalParts: [commentPart] }); + + expect(sentParts).toEqual([ + { text: BTW_BOUNDARY_INSTRUCTION, synthetic: true }, + commentPart, + ]); + }); + test('an empty parent produces a marker without a boundary', async () => { forkSessionImpl = () => Promise.resolve(makeSession('fork-1', '/project')); getSessionMessagesImpl = () => Promise.resolve([]); diff --git a/packages/ui/src/lib/btw.ts b/packages/ui/src/lib/btw.ts index dd5e1233..f56fc59d 100644 --- a/packages/ui/src/lib/btw.ts +++ b/packages/ui/src/lib/btw.ts @@ -6,6 +6,8 @@ import { useBtwStore } from '@/stores/useBtwStore'; import { useSessionUIStore } from '@/sync/session-ui-store'; import { getSyncChildStores, getSyncMessages, registerSessionDirectory } from '@/sync/sync-refs'; import { Binary } from '@/sync/binary'; +import type { ContextPartMetadata } from '@/lib/messages/contextParts'; +import type { AttachedFile } from '@/stores/types/sessionTypes'; /** * `/btw `: fork the main session into a temporary session and send @@ -28,6 +30,13 @@ export type StartBtwInput = { modelID: string; agent?: string; variant?: string; + attachments?: AttachedFile[]; + additionalParts?: Array<{ + text: string; + attachments?: AttachedFile[]; + synthetic?: boolean; + metadata?: ContextPartMetadata; + }>; }; /** @@ -196,12 +205,12 @@ export async function startBtwSession(input: StartBtwInput): Promise { input.providerID, input.modelID, input.agent, - [], + input.attachments ?? [], 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(), + [...btwBoundaryParts(), ...(input.additionalParts ?? [])], input.variant, 'normal', { sessionId: forked.id, directory: sessionDirectory }, diff --git a/packages/ui/src/lib/messages/contextParts.test.ts b/packages/ui/src/lib/messages/contextParts.test.ts index a04b8658..137fb250 100644 --- a/packages/ui/src/lib/messages/contextParts.test.ts +++ b/packages/ui/src/lib/messages/contextParts.test.ts @@ -121,6 +121,43 @@ describe('round-trip through part metadata', () => { expect(readContextPart(part)).toEqual(payload); }); + test('code comments also carry OpenCode Desktop metadata', () => { + const payload = contextPayloadFromDraft(draft()); + const part = asPart(payload); + expect(part.metadata.opencodeComment).toEqual({ + path: 'src/app.ts', + selection: { startLine: 3, endLine: 5, startChar: 0, endChar: 0 }, + comment: 'fix this', + preview: 'const x = 1;', + origin: 'review', + }); + expect(readContextPart(part)).toEqual(payload); + }); + + test('reads OpenCode Desktop metadata when canonical metadata is absent', () => { + expect(readContextPart({ + type: 'text', + metadata: { + opencodeComment: { + path: 'src/other.ts', + selection: { startLine: 8, endLine: 9 }, + comment: 'check this', + preview: 'value', + origin: 'review', + }, + }, + })).toEqual({ + kind: 'code-comment', + source: 'diff', + fileLabel: 'src/other.ts', + startLine: 8, + endLine: 9, + language: '', + code: 'value', + text: 'check this', + }); + }); + test('non-text parts, missing metadata, and malformed payloads read as null', () => { expect(readContextPart({ type: 'file', metadata: {} })).toBeNull(); expect(readContextPart({ type: 'text' })).toBeNull(); diff --git a/packages/ui/src/lib/messages/contextParts.ts b/packages/ui/src/lib/messages/contextParts.ts index 1570da37..e94415f7 100644 --- a/packages/ui/src/lib/messages/contextParts.ts +++ b/packages/ui/src/lib/messages/contextParts.ts @@ -20,6 +20,7 @@ import type { InlineCommentDraft } from '@/stores/useInlineCommentDraftStore'; import { appendTerminalContexts } from './terminalContext'; export const CONTEXT_METADATA_KEY = 'openchamberContext'; +const OPENCODE_COMMENT_METADATA_KEY = 'opencodeComment'; export type CodeCommentContext = { kind: 'code-comment'; @@ -115,7 +116,18 @@ export type ContextPartPayload = | GitHubPrContext | LinearIssueContext; -export type ContextPartMetadata = { [K in typeof CONTEXT_METADATA_KEY]: ContextPartPayload }; +type OpenCodeCommentMetadata = { + path: string; + selection?: { startLine: number; endLine: number; startChar?: number; endChar?: number }; + comment: string; + preview?: string; + origin?: 'file' | 'review'; +}; + +export type ContextPartMetadata = { + [CONTEXT_METADATA_KEY]: ContextPartPayload; + [OPENCODE_COMMENT_METADATA_KEY]?: OpenCodeCommentMetadata; +}; export type ContextPart = { text: string; @@ -177,10 +189,25 @@ export function formatContextText(payload: ContextPartPayload): string { */ export function createContextPart(payload: ContextPartPayload, text?: string): ContextPart { const resolvedText = text ?? formatContextText(payload); + const metadata: ContextPartMetadata = { [CONTEXT_METADATA_KEY]: payload }; + if (payload.kind === 'code-comment') { + metadata[OPENCODE_COMMENT_METADATA_KEY] = { + path: payload.fileLabel, + selection: { + startLine: payload.startLine, + endLine: payload.endLine, + startChar: 0, + endChar: 0, + }, + comment: payload.text, + preview: payload.code, + origin: payload.source === 'diff' ? 'review' : 'file', + }; + } return { text: resolvedText, synthetic: true, - metadata: { [CONTEXT_METADATA_KEY]: payload }, + metadata, }; } @@ -319,7 +346,28 @@ const contextPayloadSchema = z.discriminatedUnion('kind', [ * Part metadata carrying a context payload, for parsing at a trust boundary * (a queued message coming back from the server, for instance). */ -export const contextPartMetadataSchema = z.object({ [CONTEXT_METADATA_KEY]: contextPayloadSchema }); +const openCodeCommentSchema = z.object({ + path: z.string(), + selection: z.object({ + startLine: z.number().finite(), + endLine: z.number().finite(), + startChar: z.number().finite().optional(), + endChar: z.number().finite().optional(), + }).optional(), + comment: z.string(), + preview: z.string().optional(), + origin: z.enum(['file', 'review']).optional(), +}); + +/** + * Part metadata carrying a context payload, for parsing at a trust boundary + * (a queued message coming back from the server, for instance). The OpenCode + * Desktop mirror rides along so a queued comment keeps it too. + */ +export const contextPartMetadataSchema = z.object({ + [CONTEXT_METADATA_KEY]: contextPayloadSchema, + [OPENCODE_COMMENT_METADATA_KEY]: openCodeCommentSchema.optional(), +}); /** The subset of a message part that context read-back inspects. */ export type ContextCarrierPart = { type: string } & Pick; @@ -332,7 +380,32 @@ export type ContextCarrierPart = { type: string } & Pick; export function readContextPart(part: ContextCarrierPart): ContextPartPayload | null { if (part.type !== 'text') return null; const parsed = contextPayloadSchema.safeParse(part.metadata?.[CONTEXT_METADATA_KEY]); - return parsed.success ? parsed.data : null; + if (parsed.success) return parsed.data; + + const compatible = openCodeCommentSchema.safeParse(part.metadata?.[OPENCODE_COMMENT_METADATA_KEY]); + if (compatible.success) { + const comment = compatible.data; + if (!comment.selection) { + return { + kind: 'file-quote', + fileLabel: comment.path, + quote: comment.preview ?? '', + text: comment.comment, + }; + } + return { + kind: 'code-comment', + source: comment.origin === 'review' ? 'diff' : 'file', + fileLabel: comment.path, + startLine: comment.selection.startLine, + endLine: comment.selection.endLine, + language: '', + code: comment.preview ?? '', + text: comment.comment, + }; + } + + return null; } /** Whether a message carries any user-attached context part. */ diff --git a/packages/ui/src/stores/useInlineCommentDraftStore.callerId.test.ts b/packages/ui/src/stores/useInlineCommentDraftStore.callerId.test.ts new file mode 100644 index 00000000..b1ca66c4 --- /dev/null +++ b/packages/ui/src/stores/useInlineCommentDraftStore.callerId.test.ts @@ -0,0 +1,67 @@ +import { afterEach, describe, expect, test } from 'bun:test'; +import { useInlineCommentDraftStore } from './useInlineCommentDraftStore'; + +const comment = { + source: 'file' as const, + fileLabel: 'src/app.ts:12', + startLine: 12, + endLine: 12, + code: 'const x = 1', + language: 'typescript', + text: 'fix this', +}; +const target = { directory: '/repo', sessionKey: 'session-1' }; + +const store = () => useInlineCommentDraftStore.getState(); + +describe('caller-provided draft ids', () => { + afterEach(() => { useInlineCommentDraftStore.setState({ drafts: {}, touchedAt: {} }); }); + + test('a caller that owns its own view of the draft chooses the id', () => { + // The VS Code editor thread mints the id so it can track its draft without + // waiting for a round trip. + const id = store().addDraft(target, { ...comment, id: 'icd-editor-thread' }); + expect(id).toBe('icd-editor-thread'); + expect(store().getDrafts(target)[0].id).toBe('icd-editor-thread'); + }); + + test('the chosen id is what removal and lookup accept', () => { + store().addDraft(target, { ...comment, id: 'icd-editor-thread' }); + store().removeDraft(target, 'icd-editor-thread'); + expect(store().getDrafts(target)).toEqual([]); + }); + + test('omitting the id still generates one', () => { + const id = store().addDraft(target, comment); + expect(/^icd-\d+-\w+$/.test(id ?? '')).toBe(true); + }); + + test('a blank id is ignored rather than stored', () => { + const id = store().addDraft(target, { ...comment, id: ' ' }); + expect(/^icd-\d+-\w+$/.test(id ?? '')).toBe(true); + }); + + test('a colliding id is refused, so edits cannot retarget another draft', () => { + const first = store().addDraft(target, { ...comment, id: 'icd-duplicate' }); + const second = store().addDraft(target, { ...comment, id: 'icd-duplicate', text: 'different' }); + + expect(first).toBe('icd-duplicate'); + expect(second).not.toBe('icd-duplicate'); + + const drafts = store().getDrafts(target); + expect(drafts).toHaveLength(2); + expect(new Set(drafts.map((draft) => draft.id)).size).toBe(2); + }); + + test('the same id may be reused once its draft is gone', () => { + store().addDraft(target, { ...comment, id: 'icd-reused' }); + store().removeDraft(target, 'icd-reused'); + expect(store().addDraft(target, { ...comment, id: 'icd-reused' })).toBe('icd-reused'); + }); + + test('an id taken in another session does not collide', () => { + const other = { directory: '/repo', sessionKey: 'session-2' }; + store().addDraft(target, { ...comment, id: 'icd-shared' }); + expect(store().addDraft(other, { ...comment, id: 'icd-shared' })).toBe('icd-shared'); + }); +}); diff --git a/packages/ui/src/stores/useInlineCommentDraftStore.ts b/packages/ui/src/stores/useInlineCommentDraftStore.ts index c90bc07a..6fd38e2e 100644 --- a/packages/ui/src/stores/useInlineCommentDraftStore.ts +++ b/packages/ui/src/stores/useInlineCommentDraftStore.ts @@ -36,7 +36,11 @@ interface InlineCommentDraftState { } interface InlineCommentDraftActions { - addDraft: (target: InlineCommentDraftTarget, draft: Omit) => string | null; + // Returns the new draft id, or null when the draft is rejected (unresolved + // target, bounds eviction, or an empty terminal-context selection). + // `id` lets an external owner (the VS Code editor comment thread) choose the + // draft id up front. Omitted by every in-app caller, which gets a generated one. + addDraft: (target: InlineCommentDraftTarget, draft: Omit & { id?: string }) => string | null; updateDraft: (target: InlineCommentDraftTarget, draftId: string, updates: Partial>) => void; removeDraft: (target: InlineCommentDraftTarget, draftId: string) => void; clearDrafts: (target: InlineCommentDraftTarget) => void; @@ -241,7 +245,15 @@ export const useInlineCommentDraftStore = create()( addDraft: (target, draft) => { const key = getCurrentKey(target); if (!key || (draft.source === 'terminal' && !draft.code.trim())) return null; - const id = `icd-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`; + // A caller that owns its own view of the draft (the VS Code editor + // thread) supplies the id so it can correlate without a round trip. + // A colliding id would silently retarget edits and removals at an + // unrelated draft, so it is refused rather than reused. + const requestedId = draft.id?.trim(); + const idIsTaken = Boolean(requestedId) && (get().drafts[key] ?? []).some((item) => item.id === requestedId); + const id = requestedId && !idIsTaken + ? requestedId + : `icd-${Date.now()}-${Math.random().toString(36).substring(2, 9)}`; const nextDraft: InlineCommentDraft = { ...draft, sessionKey: target.sessionKey, id, createdAt: Date.now() }; let accepted = false; set((state) => { diff --git a/packages/ui/src/sync/DOCUMENTATION.md b/packages/ui/src/sync/DOCUMENTATION.md index 2d2cc24a..ed862913 100644 --- a/packages/ui/src/sync/DOCUMENTATION.md +++ b/packages/ui/src/sync/DOCUMENTATION.md @@ -291,6 +291,7 @@ Rules: 9. `SessionLiveActivity` has three answers and `unknown` is never `idle`. `getSessionLiveActivity` reports `active` when any child store or the global session-status index holds a non-idle status, `idle` only when a child store actually covers the session's directory, and `unknown` otherwise — child stores are evicted for background directories, and the global index keeps only non-idle entries, so absence of a status is not proof of idleness. Callers that gate a destructive action (worktree moves) must refuse on `unknown`. 10. Revert and unrevert cascade through known descendant sessions before mutating the parent. Revert uses the first descendant user message at or after the parent's target timestamp, including equal timestamps because message IDs do not define chronology. A descendant failure is logged and does not block its siblings or the parent. The parent runs last so its shared-directory file snapshot remains authoritative. A busy descendant is aborted before it is reverted, like the parent, so nothing keeps writing past the revert boundary. Redo clears the revert marker on every descendant, including markers the user set on a subagent independently of the parent undo. 11. Starting a session from an assistant answer carries the source session ID, rendered directory, and answer text into the action. It must not rediscover that context from the globally active child store or the OpenCode client's fallback directory: the visible session may belong to an existing worktree while the active provider directory points elsewhere. New isolated worktrees resolve their registered parent project from that captured directory, preferring recorded worktree metadata when available. The dialog offers creation only after the project root is confirmed as a Git repository, and the creation boundary repeats that check so stale or bypassed UI state cannot run Git commands against a non-repository directory; failures leave the dialog open and visible. +12. OpenCode commands and skills keep the authoritative `session.command` route when their only additional part is explicitly tagged session knowledge. Every other additional part, including unstructured synthetic conflict instructions, requires the prompt route; primary file attachments remain supported by `session.command`. Because session knowledge cannot be forwarded through the command route, it remains pending for the session's next prompt instead of being marked as delivered. Examples of global-store updates performed in `session-actions.ts`: diff --git a/packages/ui/src/sync/session-ui-store.test.js b/packages/ui/src/sync/session-ui-store.test.js index 4dcb50a8..8946618b 100644 --- a/packages/ui/src/sync/session-ui-store.test.js +++ b/packages/ui/src/sync/session-ui-store.test.js @@ -13,6 +13,7 @@ import { getRuntimeKey } from '@/lib/runtime-switch'; import { getDeferredSafeStorage } from '@/stores/utils/safeStorage'; import { CHAT_DRAFT_PROJECT_ID } from '@/lib/chatDirectories'; import { useSessionDisplayStore } from '@/stores/useSessionDisplayStore'; +import { createContextPart } from '@/lib/messages/contextParts'; /** * Unit tests for session worktree routing through the authoritative store. @@ -956,6 +957,135 @@ describe('routeMessage skill invocation', () => { expect(sendCommandCalls[0].arguments).toBe('focus on auth'); }); + test('preserves context parts and skill invocation on the prompt route', async () => { + useSkillsStore.setState({ + skills: [{ name: 'grill-with-docs', path: '/skills/grill-with-docs/SKILL.md', scope: 'user', source: 'opencode' }], + }); + const additionalParts = [createContextPart({ + kind: 'code-comment', + source: 'file', + fileLabel: 'src/auth.ts', + startLine: 4, + endLine: 4, + language: 'ts', + code: 'auth();', + text: 'check this', + })]; + + await routeMessage({ + sessionId: 'session-skill', + directory: '/skills/project', + content: '/grill-with-docs focus on auth', + providerID: 'provider-a', + modelID: 'model-a', + additionalParts, + }); + + expect(sendCommandCalls).toHaveLength(0); + expect(sendMessageCalls).toHaveLength(1); + expect(sendMessageCalls[0].additionalParts[0]).toEqual(additionalParts[0]); + expect(sendMessageCalls[0].additionalParts[1]).toMatchObject({ synthetic: true }); + expect(sendMessageCalls[0].additionalParts[1].text).toContain('grill-with-docs skill'); + }); + + test('expands a contextual command template on the prompt route', async () => { + useCommandsStore.setState({ + commands: [{ name: 'inspect', template: 'Inspect $ARGUMENTS carefully.' }], + }); + + await routeMessage({ + sessionId: 'session-command', + directory: '/skills/project', + content: '/inspect auth flow', + providerID: 'provider-a', + modelID: 'model-a', + additionalParts: [createContextPart({ + kind: 'code-comment', + source: 'file', + fileLabel: 'src/auth.ts', + startLine: 4, + endLine: 4, + language: 'ts', + code: 'auth();', + text: 'check this', + })], + }); + + expect(sendCommandCalls).toHaveLength(0); + expect(sendMessageCalls).toHaveLength(1); + expect(sendMessageCalls[0].text).toBe('Inspect auth flow carefully.'); + expect(sendMessageCalls[0].additionalParts[0].metadata.openchamberContext.kind).toBe('code-comment'); + }); + + test('keeps session.command when the only extra part is pinned knowledge', async () => { + useSkillsStore.setState({ + skills: [{ name: 'grill-with-docs', path: '/skills/grill-with-docs/SKILL.md', scope: 'user', source: 'opencode' }], + }); + + const route = await routeMessage({ + sessionId: 'session-skill', + directory: '/skills/project', + content: '/grill-with-docs focus on auth', + providerID: 'provider-a', + modelID: 'model-a', + additionalParts: [{ text: 'Pinned project knowledge', synthetic: true, systemContext: 'session-knowledge' }], + }); + + expect(sendCommandCalls).toHaveLength(1); + expect(sendCommandCalls[0].command).toBe('grill-with-docs'); + expect(sendCommandCalls[0].arguments).toBe('focus on auth'); + expect(sendMessageCalls).toHaveLength(0); + expect(route).toBe('command'); + }); + + test('keeps primary file attachments on the command route', async () => { + useSkillsStore.setState({ + skills: [{ name: 'grill-with-docs', path: '/skills/grill-with-docs/SKILL.md', scope: 'user', source: 'opencode' }], + }); + const files = [{ + type: 'file', + mime: 'text/plain', + url: 'file:///projects/alpha/auth.txt', + filename: 'auth.txt', + }]; + + const route = await routeMessage({ + sessionId: 'session-skill', + directory: '/skills/project', + content: '/grill-with-docs focus on auth', + providerID: 'provider-a', + modelID: 'model-a', + files, + additionalParts: [{ text: 'Pinned project knowledge', synthetic: true, systemContext: 'session-knowledge' }], + }); + + expect(route).toBe('command'); + expect(sendCommandCalls).toHaveLength(1); + expect(sendCommandCalls[0].files).toEqual(files); + expect(sendMessageCalls).toHaveLength(0); + }); + + test('keeps unmarked synthetic instructions on the prompt route', async () => { + useSkillsStore.setState({ + skills: [{ name: 'grill-with-docs', path: '/skills/grill-with-docs/SKILL.md', scope: 'user', source: 'opencode' }], + }); + const instructions = [{ text: 'Resolve the prepared conflict first.', synthetic: true }]; + + const route = await routeMessage({ + sessionId: 'session-skill', + directory: '/skills/project', + content: '/grill-with-docs focus on auth', + providerID: 'provider-a', + modelID: 'model-a', + additionalParts: instructions, + }); + + expect(route).toBe('prompt'); + expect(sendCommandCalls).toHaveLength(0); + expect(sendMessageCalls).toHaveLength(1); + expect(sendMessageCalls[0].additionalParts[0]).toEqual(instructions[0]); + }); + test('sends an unknown slash token as a plain message', async () => { await routeMessage({ sessionId: 'session-skill', diff --git a/packages/ui/src/sync/session-ui-store.ts b/packages/ui/src/sync/session-ui-store.ts index 1f0414f6..4c3dd05b 100644 --- a/packages/ui/src/sync/session-ui-store.ts +++ b/packages/ui/src/sync/session-ui-store.ts @@ -126,7 +126,7 @@ export function expandSlashCommandGoalObjective(content: string, commands: GoalC // Send routing — shell mode, slash commands, or normal prompt // --------------------------------------------------------------------------- -export function routeMessage(params: { +export async function routeMessage(params: { runtimeKey?: string sessionId: string directory?: string | null @@ -138,19 +138,22 @@ export function routeMessage(params: { variant?: string inputMode?: "normal" | "shell" files?: Array<{ type: "file"; mime: string; url: string; filename: string }> - additionalParts?: Array<{ text: string; synthetic?: boolean; metadata?: ContextPartMetadata; files?: Array<{ type: "file"; mime: string; url: string; filename: string }> }> + additionalParts?: Array<{ text: string; synthetic?: boolean; metadata?: ContextPartMetadata; files?: Array<{ type: "file"; mime: string; url: string; filename: string }>; systemContext?: 'session-knowledge' }> delivery?: 'steer' -}): Promise { +}): Promise<'command' | 'prompt' | 'shell'> { const requestDirectory = params.directory ?? undefined + let promptContent = params.content + let promptAdditionalParts = params.additionalParts if (params.inputMode === "shell") { - return opencodeClient.shellSession({ + await opencodeClient.shellSession({ runtimeKey: params.runtimeKey, sessionId: params.sessionId, directory: requestDirectory, agent: params.agent ?? "", model: { providerID: params.providerID, modelID: params.modelID }, command: params.content, - }).then(() => undefined) + }) + return 'shell' } // Slash commands — fire and forget, SSE delivers messages and status @@ -165,41 +168,65 @@ export function routeMessage(params: { // OpenCode registers every skill as a command (source: "skill"), but the // commands store filters skills out and the synced command list is only // hydrated at bootstrap. Consult the live skills store so a skill selected - // from the slash menu is invoked via session.command (injecting its - // content) instead of being sent as a literal "/name" message (#1605). - const isCommand = syncCommands.find((c) => c.name === cmdName) + // from the slash menu keeps its invocation semantics (#1605). + const matchedCommand = syncCommands.find((c) => c.name === cmdName) || storeCommands.find((c) => c.name === cmdName) - || useSkillsStore.getState().skills.some((s) => s.name === cmdName) + const matchedSkill = useSkillsStore.getState().skills.find((s) => s.name === cmdName) - if (isCommand) { - return optimisticSend({ - runtimeKey: params.runtimeKey, - sessionId: params.sessionId, - content: params.content, - providerID: params.providerID, - modelID: params.modelID, - agent: params.agent, - directory: requestDirectory, - files: params.files, - send: (messageID) => opencodeClient.sendCommand({ + if (matchedCommand || matchedSkill) { + // Pinned project knowledge is the only additional part that does not + // change command semantics. Other synthetic parts may carry prepared + // user work (for example conflict instructions) and must not be dropped. + const additionalPartsRequirePrompt = params.additionalParts?.some((part) => ( + part.systemContext !== 'session-knowledge' + )) ?? false + if (!additionalPartsRequirePrompt) { + await optimisticSend({ runtimeKey: params.runtimeKey, - id: params.sessionId, + sessionId: params.sessionId, + content: params.content, providerID: params.providerID, modelID: params.modelID, - command: cmdName, - arguments: tail.join(" "), agent: params.agent, - variant: params.variant, - files: params.files, - messageId: messageID, directory: requestDirectory, - }).then(() => {}), - }) + files: params.files, + send: (messageID) => opencodeClient.sendCommand({ + runtimeKey: params.runtimeKey, + id: params.sessionId, + providerID: params.providerID, + modelID: params.modelID, + command: cmdName, + arguments: tail.join(" "), + agent: params.agent, + variant: params.variant, + files: params.files, + messageId: messageID, + directory: requestDirectory, + }).then(() => {}), + }) + return 'command' + } + + // session.command accepts file parts only. Keep structured context on + // the prompt route, expanding templates locally when available and + // preserving skill invocation as an explicit synthetic instruction. + if (matchedCommand?.template?.trim()) { + promptContent = expandSlashCommandGoalObjective(params.content, [matchedCommand]) + } + if (matchedSkill) { + promptAdditionalParts = [ + ...(params.additionalParts ?? []), + { + text: `The user explicitly invoked the ${cmdName} skill. Use the corresponding skill tool to handle this request.`, + synthetic: true, + }, + ] + } } } // Normal prompt — optimistic insert so message appears instantly - return optimisticSend({ + await optimisticSend({ runtimeKey: params.runtimeKey, sessionId: params.sessionId, content: params.content, @@ -213,17 +240,23 @@ export function routeMessage(params: { id: params.sessionId, providerID: params.providerID, modelID: params.modelID, - text: params.content, + text: promptContent, agent: params.agent, agentMentions: params.agentMentionName ? [{ name: params.agentMentionName }] : undefined, variant: params.variant, files: params.files, - additionalParts: params.additionalParts, + additionalParts: promptAdditionalParts?.map((part) => ({ + text: part.text, + synthetic: part.synthetic, + metadata: part.metadata, + files: part.files, + })), delivery: params.delivery, messageId: messageID, directory: requestDirectory, }).then(() => {}), }) + return 'prompt' } type CapturedSendTarget = { @@ -365,7 +398,7 @@ export type SessionUIState = { agent?: string, attachments?: AttachedFile[], agentMentionName?: string, - additionalParts?: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata }>, + additionalParts?: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata; systemContext?: 'session-knowledge' }>, variant?: string, inputMode?: "normal" | "shell", options?: SendMessageOptions, @@ -1583,7 +1616,7 @@ export const useSessionUIStore = create()((set, get) => ({ agent?: string, attachments?: AttachedFile[], agentMentionName?: string, - additionalParts?: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata }>, + additionalParts?: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata; systemContext?: 'session-knowledge' }>, variant?: string, inputMode?: "normal" | "shell", options?: SendMessageOptions, @@ -1662,7 +1695,7 @@ export const useSessionUIStore = create()((set, get) => ({ }, options?.draftSnapshot) if (!createdDraftSession) throw new Error("Failed to create session") - const draftParts = createdDraftSession.syntheticParts?.length + const draftParts: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata; systemContext?: 'session-knowledge' }> | undefined = createdDraftSession.syntheticParts?.length ? [...(additionalParts || []), ...createdDraftSession.syntheticParts] : additionalParts // The server decides what this session still owes and assembles it; the @@ -1671,8 +1704,8 @@ export const useSessionUIStore = create()((set, get) => ({ createdDraftSession.directory, createdDraftSession.sessionId, ) - const draftPrefixParts: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata }> = - draftKnowledge.text ? [{ text: draftKnowledge.text, synthetic: true }] : [] + const draftPrefixParts: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata; systemContext?: 'session-knowledge' }> = + draftKnowledge.text ? [{ text: draftKnowledge.text, synthetic: true, systemContext: 'session-knowledge' }] : [] // Left undefined when nothing was added, as before: an empty array is not // the same as no additional parts to everything downstream. const mergedAdditionalParts = draftPrefixParts.length > 0 @@ -1691,7 +1724,7 @@ export const useSessionUIStore = create()((set, get) => ({ })) await applyArmedGoal(createdDraftSession.sessionId, createdDraftSession.directory) - await routeMessage({ + const messageRoute = await routeMessage({ sessionId: createdDraftSession.sessionId, directory: createdDraftSession.directory, content, @@ -1707,6 +1740,7 @@ export const useSessionUIStore = create()((set, get) => ({ text: p.text, synthetic: p.synthetic, metadata: p.metadata, + systemContext: p.systemContext, files: p.attachments?.map((a: AttachedFile) => ({ type: "file" as const, mime: a.mimeType, @@ -1717,7 +1751,7 @@ export const useSessionUIStore = create()((set, get) => ({ }) // Recorded only after the send resolves: a failed send must carry the // pinned context again rather than assume the agent already saw it. - if (draftKnowledge.text) { + if (draftKnowledge.text && messageRoute === 'prompt') { void reportSessionKnowledgeDelivered( createdDraftSession.directory, createdDraftSession.sessionId, @@ -1794,13 +1828,13 @@ export const useSessionUIStore = create()((set, get) => ({ // Prepended so it reads as background before the message it accompanies, // and empty unless the session is actually missing it. const knowledge = await fetchSessionKnowledge(currentSessionDirectory, targetSessionId || "") - const prefixParts: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata }> = - knowledge.text ? [{ text: knowledge.text, synthetic: true }] : [] + const prefixParts: Array<{ text: string; attachments?: AttachedFile[]; synthetic?: boolean; metadata?: ContextPartMetadata; systemContext?: 'session-knowledge' }> = + knowledge.text ? [{ text: knowledge.text, synthetic: true, systemContext: 'session-knowledge' }] : [] const partsWithPinnedContext = prefixParts.length > 0 ? [...prefixParts, ...(additionalParts || [])] : additionalParts - await routeMessage({ + const messageRoute = await routeMessage({ runtimeKey: capturedTarget?.runtimeKey, sessionId: targetSessionId || "", directory: currentSessionDirectory, @@ -1817,6 +1851,7 @@ export const useSessionUIStore = create()((set, get) => ({ text: p.text, synthetic: p.synthetic, metadata: p.metadata, + systemContext: p.systemContext, files: p.attachments?.map((a) => ({ type: "file" as const, mime: a.mimeType, @@ -1825,7 +1860,7 @@ export const useSessionUIStore = create()((set, get) => ({ })), })), }) - if (knowledge.text) { + if (knowledge.text && messageRoute === 'prompt') { void reportSessionKnowledgeDelivered(currentSessionDirectory, targetSessionId || "", knowledge.signature) } }, diff --git a/packages/vscode/README.md b/packages/vscode/README.md index 39c87f3e..f3f58081 100644 --- a/packages/vscode/README.md +++ b/packages/vscode/README.md @@ -42,6 +42,7 @@ Select code in the editor, right-click, and find the **OpenChamber** submenu: | Action | Description | |--------|-------------| | Add to Context | Attach selection to your next prompt | +| Add Comment | Open a comment thread on the selected lines. The comment stays anchored in the editor and goes out with your next message as a context card. The gutter `+` does the same once the extension is active | | Explain | Ask the agent to explain the selected code | | Improve Code | Ask the agent to improve the selection in-place | diff --git a/packages/vscode/l10n/bundle.l10n.fr.json b/packages/vscode/l10n/bundle.l10n.fr.json index 03807689..d6144f70 100644 --- a/packages/vscode/l10n/bundle.l10n.fr.json +++ b/packages/vscode/l10n/bundle.l10n.fr.json @@ -13,6 +13,13 @@ "OpenChamber [Improve Code]: No active editor": "OpenChamber [Améliorer le code] : aucun éditeur actif", "OpenChamber [Improve Code]: No text selected": "OpenChamber [Améliorer le code] : aucun texte sélectionné", "Improve the following Code:": "Améliorez le code suivant :", + "OpenChamber [Add Comment]: No active editor": "OpenChamber [Ajouter un commentaire] : aucun éditeur actif", + "OpenChamber [Add Comment]: File is outside the workspace": "OpenChamber [Ajouter un commentaire] : le fichier est en dehors de l’espace de travail", + "OpenChamber [Add Comment]: The comment never reached the chat and was discarded": "OpenChamber [Ajouter un commentaire] : le commentaire n’est jamais arrivé dans la discussion et a été abandonné", + "Comment on line {0}": "Commentaire sur la ligne {0}", + "Comment on lines {0}-{1}": "Commentaire sur les lignes {0} à {1}", + "OpenChamber": "OpenChamber", + "Not sent yet": "Pas encore envoyé", "OpenCode CLI not found. Install it and ensure it's in PATH.": "OpenCode CLI introuvable. Installez-le et assurez-vous qu’il est dans le PATH.", "OpenCode CLI not found. Please install it and ensure it's in PATH.": "OpenCode CLI introuvable. Veuillez l’installer et vérifier qu’il est dans le PATH.", "More Info": "Plus d’informations", diff --git a/packages/vscode/l10n/bundle.l10n.json b/packages/vscode/l10n/bundle.l10n.json index 3469cc4c..7b356479 100644 --- a/packages/vscode/l10n/bundle.l10n.json +++ b/packages/vscode/l10n/bundle.l10n.json @@ -13,6 +13,13 @@ "OpenChamber [Improve Code]: No active editor": "OpenChamber [Improve Code]: No active editor", "OpenChamber [Improve Code]: No text selected": "OpenChamber [Improve Code]: No text selected", "Improve the following Code:": "Improve the following Code:", + "OpenChamber [Add Comment]: No active editor": "OpenChamber [Add Comment]: No active editor", + "OpenChamber [Add Comment]: File is outside the workspace": "OpenChamber [Add Comment]: File is outside the workspace", + "OpenChamber [Add Comment]: The comment never reached the chat and was discarded": "OpenChamber [Add Comment]: The comment never reached the chat and was discarded", + "Comment on line {0}": "Comment on line {0}", + "Comment on lines {0}-{1}": "Comment on lines {0}-{1}", + "OpenChamber": "OpenChamber", + "Not sent yet": "Not sent yet", "OpenCode CLI not found. Install it and ensure it's in PATH.": "OpenCode CLI not found. Install it and ensure it's in PATH.", "OpenCode CLI not found. Please install it and ensure it's in PATH.": "OpenCode CLI not found. Please install it and ensure it's in PATH.", "More Info": "More Info", diff --git a/packages/vscode/package.json b/packages/vscode/package.json index ca1753c3..fc3474ef 100644 --- a/packages/vscode/package.json +++ b/packages/vscode/package.json @@ -115,6 +115,22 @@ "category": "OpenChamber", "title": "%command.addToContext.title%" }, + { + "command": "openchamber.addLineComment", + "category": "OpenChamber", + "title": "%command.addLineComment.title%" + }, + { + "command": "openchamber.submitLineComment", + "category": "OpenChamber", + "title": "%command.submitLineComment.title%" + }, + { + "command": "openchamber.removeLineComment", + "category": "OpenChamber", + "title": "%command.removeLineComment.title%", + "icon": "$(close)" + }, { "command": "openchamber.explain", "category": "OpenChamber", @@ -150,6 +166,30 @@ } ], "menus": { + "comments/commentThread/context": [ + { + "command": "openchamber.submitLineComment", + "group": "inline", + "when": "commentController == openchamber.inlineComments" + } + ], + "comments/commentThread/title": [ + { + "command": "openchamber.removeLineComment", + "group": "navigation", + "when": "commentController == openchamber.inlineComments && commentThread == openchamberAttached" + } + ], + "commandPalette": [ + { + "command": "openchamber.submitLineComment", + "when": "false" + }, + { + "command": "openchamber.removeLineComment", + "when": "false" + } + ], "editor/context": [ { "submenu": "openchamber.submenu", @@ -200,6 +240,9 @@ }, { "command": "openchamber.addToContext" + }, + { + "command": "openchamber.addLineComment" } ] }, diff --git a/packages/vscode/package.nls.fr.json b/packages/vscode/package.nls.fr.json index 0f12fb3c..718e9de2 100644 --- a/packages/vscode/package.nls.fr.json +++ b/packages/vscode/package.nls.fr.json @@ -10,6 +10,9 @@ "command.openNewSessionInEditor.title": "Ouvrir une nouvelle session dans l’éditeur", "command.openCurrentOrNewSessionInEditor.title": "Ouvrir une session dans l’éditeur", "command.addToContext.title": "Ajouter au contexte", + "command.addLineComment.title": "Ajouter un commentaire", + "command.submitLineComment.title": "Commenter", + "command.removeLineComment.title": "Supprimer le commentaire", "command.explain.title": "Expliquer", "command.improveCode.title": "Améliorer le code", "command.newSession.title": "Nouvelle session", diff --git a/packages/vscode/package.nls.json b/packages/vscode/package.nls.json index ac3a1d56..bb77411a 100644 --- a/packages/vscode/package.nls.json +++ b/packages/vscode/package.nls.json @@ -10,6 +10,9 @@ "command.openNewSessionInEditor.title": "Open New Session in Editor", "command.openCurrentOrNewSessionInEditor.title": "Open Session in Editor", "command.addToContext.title": "Add to Context", + "command.addLineComment.title": "Add Comment", + "command.submitLineComment.title": "Comment", + "command.removeLineComment.title": "Remove Comment", "command.explain.title": "Explain", "command.improveCode.title": "Improve Code", "command.newSession.title": "New Session", diff --git a/packages/vscode/src/ChatViewProvider.ts b/packages/vscode/src/ChatViewProvider.ts index ab547fa3..4c29d6ef 100644 --- a/packages/vscode/src/ChatViewProvider.ts +++ b/packages/vscode/src/ChatViewProvider.ts @@ -9,6 +9,7 @@ import { openSseProxy } from './sseProxy'; import { resolveWebviewDevServerUrl } from './webviewDevServer'; import { normalizeWindowsDriveLetter } from './pathUtils'; import { resolveWorkspaceFolders, type WorkspaceFolderCandidate } from './workspaceResolver'; +import { SIDEBAR_SURFACE_ID } from './InlineCommentThreads'; type ActiveEditorFilePayload = { filePath: string; @@ -150,6 +151,19 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { return; } + // Editor comment threads mirror the composer's drafts, so the webview + // reports every change. Handled before the id check because this is a + // one-way notification, not a bridge request awaiting a response. + if (message.type === 'inlineComments:sync') { + // Tagged with the sidebar's identity: a snapshot only speaks for the + // store that produced it, and each session panel has its own. + void vscode.commands.executeCommand('openchamber.internal.inlineCommentsSync', { + snapshot: message.payload, + surfaceId: SIDEBAR_SURFACE_ID, + }); + return; + } + if (!('id' in message) || typeof message.id !== 'string') { return; } @@ -249,6 +263,28 @@ export class ChatViewProvider implements vscode.WebviewViewProvider { }); } + public addLineComment(payload: { draftId?: string; filePath: string; relativePath: string; source: 'diff' | 'file'; side?: 'original' | 'modified'; startLine: number; endLine: number; code: string; language: string; comment: string }) { + if (!this._view) return; + // Bring the chat into view like the other capture flows do, so the chip the + // comment becomes is visible rather than waiting behind a collapsed panel. + this._view.show(true); + this._view.webview.postMessage({ + type: 'command', + command: 'addLineComment', + payload, + }); + } + + /** Drops a draft the user removed from its editor thread. */ + public removeLineComment(draftId: string) { + if (!this._view) return; + this._view.webview.postMessage({ + type: 'command', + command: 'removeLineComment', + payload: { draftId }, + }); + } + public addFileAttachments(files: Array<{ filePath: string; fileName: string; fileSize: number | null }>) { if (!this._view) { return; diff --git a/packages/vscode/src/DOCUMENTATION.md b/packages/vscode/src/DOCUMENTATION.md index 212c58fe..bc5f2481 100644 --- a/packages/vscode/src/DOCUMENTATION.md +++ b/packages/vscode/src/DOCUMENTATION.md @@ -82,6 +82,13 @@ The webview build emits each worker as one self-contained file. VS Code webviews - Owns the persisted VS Code permission auto-accept policy and its GET/PUT bridge contract. - Serializes reads and read-modify-write updates, persists a monotonic policy revision, and broadcasts the exact committed snapshot to every active OpenChamber webview. Permission replies remain foreground UI-owned because VS Code does not run the OpenChamber server runtime. +- `InlineCommentThreads.ts` + - Owns the `openchamber.inlineComments` comment controller: the gutter `+` range, the thread opened by `openchamber.addLineComment`, and every thread a submitted comment leaves anchored in the editor until the message goes out. + - A thread never owns a draft. It mints the draft id, hands the payload to a chat webview (the active or newly opened session panel, else the sidebar), and follows the webview's whole-draft-list `inlineComments:sync` snapshots: present means show, absent after having been seen means dispose. A snapshot is tagged with the surface that produced it (a panel id or `sidebar`) and only decides that surface's own threads, because every webview runs its own draft store. + - A comment the composer never confirms holding within 30 s is retracted from every surface's pending hold, its thread disposed, and the user told, so a thread cannot promise a send that will never happen. + - `inlineCommentSelection.ts` holds the pure pieces (line ranges, the diff-side and real-path resolution for `git:` documents, the pending hold, removal broadcast, thread fate) without the `vscode` import so they are unit-tested directly. + - Webview side: `webview/inlineCommentRemovals.ts` remembers removals that arrive before a delayed delivery lands, so a comment dropped while its panel was still booting does not appear as a chip later. The extension is not activated on startup for this; the right-click command activates it, and the gutter `+` appears from then on. + ## Shared webview message ordering Message and part ordering is owned by [`packages/ui/src/sync/DOCUMENTATION.md`](../../ui/src/sync/DOCUMENTATION.md#session-message-loading). The VS Code webview consumes that shared sync implementation; bridge and proxy runtimes pass OpenCode records through without adding runtime-specific ordering. diff --git a/packages/vscode/src/InlineCommentThreads.ts b/packages/vscode/src/InlineCommentThreads.ts new file mode 100644 index 00000000..2f7fdd3d --- /dev/null +++ b/packages/vscode/src/InlineCommentThreads.ts @@ -0,0 +1,333 @@ +/** + * Code comments written in the editor itself. + * + * A comment is written while looking at the code it is about, so it is captured + * where the code is: right-click a selection (or use the gutter `+`) and a + * thread opens on those lines. The thread stays anchored there, showing what + * will be sent, until the message goes out or the comment is dropped. + * + * The composer's chips remain the list of what is attached. This module is the + * editor-side view of that same list, which is why it never owns a draft: it + * mints the id, hands the draft to the webview, and disposes its thread when + * the webview reports the draft gone. The webview store stays authoritative, + * so a comment removed from the chip row cannot linger in the editor. + */ + +import * as vscode from 'vscode'; + +import { DELIVERY_CONFIRMATION_TIMEOUT_MS, canCommentOnDocument, nextDraftId, reconcileThreadFate, resolveCommentFilePath, resolveCommentOrigin, selectionLineRange, shouldAbandonUnconfirmed, shouldDisposeOnEmptyBody, snapshotOwnsThread, type CommentOrigin, type LineRange } from './inlineCommentSelection'; + +// Also written literally in package.json, which gates the thread menus with +// `commentController == openchamber.inlineComments`. JSON cannot import, so the +// two have to be kept in step by hand. +const INLINE_COMMENT_CONTROLLER_ID = 'openchamber.inlineComments'; + +export interface InlineCommentDraftPayload { + draftId: string; + filePath: string; + relativePath: string; + source: 'diff' | 'file'; + side?: 'original' | 'modified'; + startLine: number; + endLine: number; + code: string; + language: string; + comment: string; +} + +interface OpenChamberCommentThread extends vscode.CommentThread { + draftId?: string; + /** Diff identity captured while the thread's editor is authoritative. */ + commentOrigin?: CommentOrigin; + /** Last body written to this thread, so reconciliation can skip no-op renders. */ + commentBody?: string; + /** + * Whether the composer has ever reported holding this draft. + * + * Delivery is asynchronous, so a snapshot can arrive describing the moment + * before the draft landed. Absence only means "removed" once presence has + * been seen at least once. + */ + confirmed?: boolean; + /** + * The chat webview holding this comment's draft. + * + * Only that surface's snapshots can decide this thread's fate; every other + * webview has its own store where the draft never existed. + */ + surfaceId?: string; + /** Deadline for the composer to confirm it holds this draft. */ + confirmationTimer?: ReturnType; +} + +/** Identifies one chat webview: a session panel id, or the sidebar. */ +export const SIDEBAR_SURFACE_ID = 'sidebar'; + +export interface InlineCommentThreadsOptions { + /** + * Hands a finished draft to a chat webview. + * + * Returns the id of the surface that accepted it, or null when none did. + * The identity matters: only that surface's later snapshots can speak for + * this comment, because every webview holds its own draft store. + */ + submitDraft: (payload: InlineCommentDraftPayload) => Promise | string | null; + /** Asks the webview to drop a draft the user removed from the editor side. */ + removeDraft: (draftId: string) => void; + /** Tells the user a comment never reached the composer and was given up on. */ + reportUndelivered: () => void; + /** The extension's own icon, shown as the comment's avatar. */ + avatar: vscode.Uri; + /** Localized strings, injected so this module does not reach for the l10n bundle. */ + strings: { + threadLabel: (range: LineRange) => string; + author: string; + notSent: string; + }; +} + +/** + * Owns the comment controller and every thread currently on screen. + * + * Threads are keyed by draft id once submitted. Before submission a thread has + * no draft yet, so it is tracked only by the controller and disposed on cancel. + */ +export class InlineCommentThreads implements vscode.Disposable { + private readonly controller: vscode.CommentController; + private readonly threadsByDraftId = new Map(); + private readonly options: InlineCommentThreadsOptions; + + constructor(options: InlineCommentThreadsOptions) { + this.options = options; + this.controller = vscode.comments.createCommentController( + INLINE_COMMENT_CONTROLLER_ID, + 'OpenChamber', + ); + // Any line of a workspace file can take a comment; the gutter `+` + // follows from this. + this.controller.commentingRangeProvider = { + provideCommentingRanges: (document) => { + if (!this.canCommentOn(document.uri)) return []; + return [new vscode.Range(0, 0, Math.max(document.lineCount - 1, 0), 0)]; + }, + }; + } + + /** + * Whether this document can take a comment. + * + * Both entry points ask, so the gutter `+` and the right-click command + * agree: a comment is filed against a workspace-relative path, and one + * written outside the workspace would name a file that does not resolve. + */ + public canCommentOn(uri: vscode.Uri): boolean { + const filePath = resolveCommentFilePath(uri.fsPath, uri.query); + const inWorkspace = Boolean(vscode.workspace.getWorkspaceFolder(vscode.Uri.file(filePath))); + return canCommentOnDocument(uri.scheme, inWorkspace); + } + + /** Opens an empty thread on a selection, with the reply box focused. */ + public openThread(uri: vscode.Uri, range: vscode.Range): vscode.CommentThread { + const lines = selectionLineRange(range); + // SAFETY: this controller creates and owns the thread; the added fields + // are optional extension-local bookkeeping on VS Code's mutable object. + const thread = this.controller.createCommentThread(uri, range, []) as OpenChamberCommentThread; + thread.commentOrigin = this.resolveOrigin(uri); + thread.label = this.options.strings.threadLabel(lines); + thread.collapsibleState = vscode.CommentThreadCollapsibleState.Expanded; + thread.canReply = true; + thread.contextValue = 'openchamberPending'; + return thread; + } + + /** + * Turns a typed reply into a draft the composer will send. + * + * An empty body is a cancel: the thread is disposed rather than left behind + * as a comment that will never be sent. + */ + public async submitReply(reply: { thread: vscode.CommentThread; text: string }): Promise { + // SAFETY: this command is registered only for threads created by this + // controller, which are initialized as OpenChamberCommentThread above. + const thread = reply.thread as OpenChamberCommentThread; + if (shouldDisposeOnEmptyBody(reply.text)) { + this.disposeThread(thread); + return; + } + + // A thread whose range the editor dropped (the file was closed or edited + // out from under it) has nothing to anchor a comment to. + const range = thread.range; + if (!range) { + this.disposeThread(thread); + return; + } + + // Capture before the first await. Opening a git document can yield long + // enough for tab focus to move, while the thread still belongs to the + // diff pane where the user submitted it. + const origin = thread.commentOrigin ?? this.resolveOrigin(thread.uri); + thread.commentOrigin = origin; + const document = await vscode.workspace.openTextDocument(thread.uri); + const lines = selectionLineRange(range); + const draftId = nextDraftId(Date.now(), Math.random()); + + // The gutter `+` produces a thread VS Code created, which never went + // through openThread and so carries no label. Set it here so both entry + // points read the same. + thread.label = this.options.strings.threadLabel(lines); + + // Quote the pane the user commented on, but name the real file: a diff's + // original side is a `git:` document, and its raw path is not something + // the composer can match against the workspace. + const filePath = resolveCommentFilePath(thread.uri.fsPath, thread.uri.query); + const fileUri = vscode.Uri.file(filePath); + + const payload: InlineCommentDraftPayload = { + draftId, + filePath, + relativePath: vscode.workspace.asRelativePath(fileUri, false), + ...origin, + startLine: lines.startLine, + endLine: lines.endLine, + code: document.getText(range), + language: document.languageId, + comment: reply.text, + }; + + const surfaceId = await this.options.submitDraft(payload); + if (!surfaceId) { + // Nothing took the draft (no chat surface open). Leaving the thread + // would promise an attachment that does not exist. + this.disposeThread(thread); + return; + } + + thread.surfaceId = surfaceId; + thread.draftId = draftId; + thread.commentBody = reply.text; + thread.canReply = false; + thread.contextValue = 'openchamberAttached'; + thread.comments = [this.buildComment(reply.text)]; + this.threadsByDraftId.set(draftId, thread); + + // Accepting the draft is not the same as it landing. A panel whose + // webview never boots leaves this thread showing "Not sent yet" for a + // comment that will never be sent and cannot be rewritten, so it is + // given up on rather than left as a standing promise. + thread.confirmationTimer = setTimeout(() => { + thread.confirmationTimer = undefined; + if (!shouldAbandonUnconfirmed(thread.confirmed)) return; + // Retract it everywhere before saying it was discarded. Dropping only + // the thread leaves the payload in a panel's hold, so a webview that + // boots after the deadline would still file the draft and send a + // comment the user was just told had been thrown away. + this.options.removeDraft(draftId); + this.disposeThread(thread); + this.options.reportUndelivered(); + }, DELIVERY_CONFIRMATION_TIMEOUT_MS); + } + + private resolveOrigin(uri: vscode.Uri): CommentOrigin { + const activeTabInput = vscode.window.tabGroups.activeTabGroup.activeTab?.input; + if (activeTabInput instanceof vscode.TabInputTextDiff) { + return resolveCommentOrigin(uri.toString(), uri.scheme, { + original: activeTabInput.original.toString(), + modified: activeTabInput.modified.toString(), + }); + } + return resolveCommentOrigin(uri.toString(), uri.scheme); + } + + /** + * Brings the editor threads in line with what the composer actually holds. + * + * The webview sends its whole current draft list rather than individual + * events, so a dropped or reordered notification cannot leave a thread + * anchored to a comment that will never be sent. Sending the message empties + * the list, which clears every thread through the same path. + * + * Only threads this controller created are ever touched, so an unknown id in + * the snapshot (a comment written in the in-app file viewer) is ignored + * rather than treated as something to reconcile. + * + * A snapshot speaks only for the surface that sent it. Every webview keeps + * its own draft store, so a second session tab reporting an empty list says + * nothing about a comment attached to the first one. + */ + public reconcile(surfaceId: string, drafts: ReadonlyArray<{ id: string; text: string }>): void { + const byId = new Map(drafts.map((draft) => [draft.id, draft.text])); + + for (const [draftId, thread] of [...this.threadsByDraftId]) { + if (!snapshotOwnsThread(thread.surfaceId, surfaceId)) continue; + const text = byId.get(draftId); + const fate = reconcileThreadFate(text, Boolean(thread.confirmed)); + + if (fate === 'wait') continue; + if (fate === 'dispose') { + this.disposeThread(thread); + continue; + } + + thread.confirmed = true; + if (thread.confirmationTimer) { + clearTimeout(thread.confirmationTimer); + thread.confirmationTimer = undefined; + } + if (text !== undefined && thread.commentBody !== text) { + thread.commentBody = text; + thread.comments = [this.buildComment(text)]; + } + } + } + + /** + * Removes a thread from the editor side. + * + * A thread that already carries a draft has to tell the composer, or the + * chip would stay attached with nothing shown in the code. + */ + public removeThread(thread: vscode.CommentThread): void { + // SAFETY: removeThread is wired only to this controller's comment menu. + const draftId = (thread as OpenChamberCommentThread).draftId; + if (draftId) { + this.options.removeDraft(draftId); + } + this.disposeThread(thread); + } + + public dispose(): void { + for (const thread of this.threadsByDraftId.values()) { + if (thread.confirmationTimer) clearTimeout(thread.confirmationTimer); + } + this.threadsByDraftId.clear(); + this.controller.dispose(); + } + + private buildComment(body: string): vscode.Comment { + // The body is the user's own prose, not a document: rendering it as + // Markdown would eat underscores and asterisks they meant literally, + // and a comment on code is full of both. + const rendered = new vscode.MarkdownString(); + rendered.appendText(body); + + return { + body: rendered, + mode: vscode.CommentMode.Preview, + author: { name: this.options.strings.author, iconPath: this.options.avatar }, + label: this.options.strings.notSent, + contextValue: 'openchamberAttached', + }; + } + + private disposeThread(thread: OpenChamberCommentThread): void { + if (thread.confirmationTimer) { + clearTimeout(thread.confirmationTimer); + thread.confirmationTimer = undefined; + } + if (thread.draftId) { + this.threadsByDraftId.delete(thread.draftId); + } + thread.dispose(); + } +} diff --git a/packages/vscode/src/SessionEditorPanelProvider.ts b/packages/vscode/src/SessionEditorPanelProvider.ts index 6150e13d..32bfbced 100644 --- a/packages/vscode/src/SessionEditorPanelProvider.ts +++ b/packages/vscode/src/SessionEditorPanelProvider.ts @@ -9,12 +9,40 @@ import { openSseProxy } from './sseProxy'; import { resolveWebviewDevServerUrl } from './webviewDevServer'; import { normalizeWindowsDriveLetter } from './pathUtils'; import { resolveWorkspaceFolders } from './workspaceResolver'; +import { pickActivePanelId } from './activePanelRouting'; +import { broadcastRemoval, drainPending } from './inlineCommentSelection'; const t = vscode.l10n.t; +type LineCommentPayload = { + draftId?: string; + filePath: string; + relativePath: string; + source: 'diff' | 'file'; + side?: 'original' | 'modified'; + startLine: number; + endLine: number; + code: string; + language: string; + comment: string; +}; + type SessionPanelState = { + /** This panel's id, which is also its surface identity for comment threads. */ + id: string; panel: vscode.WebviewPanel; sseStreams: Map; + /** + * Comments held until the webview proves it is listening. Posting into a + * panel whose script has not booted drops the message outright, and the user + * already saw the comment accepted. + * + * A list, because a second comment can be written while the panel is still + * starting; a single slot silently discarded the first. + */ + pendingLineComments: LineCommentPayload[]; + /** Set by the panel's first inbound message, the only proof its script runs. */ + webviewReady?: boolean; }; type ActiveEditorFilePayload = { @@ -119,8 +147,10 @@ export class SessionEditorPanelProvider { }; const state: SessionPanelState = { + id: panelId, panel, sseStreams: new Map(), + pendingLineComments: [], }; this._panels.set(panelId, state); @@ -146,6 +176,29 @@ export class SessionEditorPanelProvider { }, null, this._context.subscriptions); panel.webview.onDidReceiveMessage(async (message: BridgeRequest) => { + // Any inbound message proves the webview script is running, which is the + // only readiness signal this panel has. Flush whatever was held for it. + state.webviewReady = true; + for (const pending of drainPending(state.pendingLineComments)) { + void panel.webview.postMessage({ + type: 'command', + command: 'addLineComment', + payload: pending, + }); + } + + // Editor comment threads mirror the composer's drafts, so the webview + // reports every change. One-way notification, no response expected. + if (message.type === 'inlineComments:sync') { + // Tagged with this panel's identity: a snapshot only speaks for the + // store that produced it, and every panel has its own. + void vscode.commands.executeCommand('openchamber.internal.inlineCommentsSync', { + snapshot: message.payload, + surfaceId: panelId, + }); + return; + } + if (message.type === 'restartApi') { await this._openCodeManager?.restart(); return; @@ -246,8 +299,10 @@ export class SessionEditorPanelProvider { } private _getActivePanelEntry(): SessionPanelState | null { - const activeEntry = Array.from(this._panels.entries()).find(([, entry]) => entry.panel.active); - const panelId = activeEntry?.[0] ?? this._lastActivePanelId; + const panelId = pickActivePanelId( + Array.from(this._panels.entries()).map(([id, entry]) => ({ id, active: entry.panel.active })), + this._lastActivePanelId, + ); if (!panelId) { return null; } @@ -274,6 +329,105 @@ export class SessionEditorPanelProvider { return true; } + public addLineCommentToActivePanel(payload: { + draftId?: string; + filePath: string; + relativePath: string; + source: 'diff' | 'file'; + side?: 'original' | 'modified'; + startLine: number; + endLine: number; + code: string; + language: string; + comment: string; + }): string | null { + if (!payload.relativePath.trim()) { + return null; + } + + const entry = this._getActivePanelEntry(); + if (!entry) { + return null; + } + + entry.panel.reveal(entry.panel.viewColumn ?? vscode.ViewColumn.Active, true); + + // An existing panel can still be booting (reopened from a restored window), + // and a post into a webview whose script has not run is dropped outright. + // Hold it on the same path a freshly opened panel uses. + if (!entry.webviewReady) { + entry.pendingLineComments.push(payload); + return entry.id; + } + + void entry.panel.webview.postMessage({ + type: 'command', + command: 'addLineComment', + payload, + }); + return entry.id; + } + + /** + * Delivers a comment to a session tab, opening one when none exists. + * + * A comment is written against code the user is reading, so it must not + * depend on their having opened a chat first. With no tab open this behaves + * like the toolbar's new-session button, then delivers into that tab once its + * webview is listening. + */ + public openWithLineComment(payload: LineCommentPayload, activeSessionId: string | null): string | null { + if (!payload.relativePath.trim()) { + return null; + } + + const accepted = this.addLineCommentToActivePanel(payload); + if (accepted) { + return accepted; + } + + if (activeSessionId) { + this.createOrShow(activeSessionId); + } else { + this.createOrShowNewSession(); + } + + const entry = this._getActivePanelEntry(); + if (!entry) { + return null; + } + + entry.pendingLineComments.push(payload); + return entry.id; + } + + /** + * Drops a draft the user removed from its editor thread. + * + * Sent to every panel, not just the active one: each webview owns its own + * draft store, and the draft may have landed in a tab the user has since + * moved away from. Targeting only the active panel made removal a silent + * no-op in that case, leaving the chip attached after its thread was gone. + * + * Unlike adding, this does not reveal a panel: the user is looking at the + * code, and stealing focus to show a chip disappearing would be worse than + * letting it disappear quietly. + */ + public removeLineComment(draftId: string): void { + const targets = [...this._panels.values()].map((state) => ({ + pendingLineComments: state.pendingLineComments, + notify: () => { + void state.panel.webview.postMessage({ + type: 'command', + command: 'removeLineComment', + payload: { draftId }, + }); + }, + })); + + broadcastRemoval(targets, draftId); + } + public createSessionWithPromptInActivePanel(prompt: string): boolean { if (!prompt.trim()) { return false; diff --git a/packages/vscode/src/activePanelRouting.test.ts b/packages/vscode/src/activePanelRouting.test.ts new file mode 100644 index 00000000..30624148 --- /dev/null +++ b/packages/vscode/src/activePanelRouting.test.ts @@ -0,0 +1,39 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { pickActivePanelId } from './activePanelRouting'; + +describe('pickActivePanelId', () => { + test('returns null when there are no panels and no recent panel', () => { + assert.equal(pickActivePanelId([], null), null); + }); + + test('falls back to the last active panel when none is currently focused', () => { + // A chat panel exists but the user is focused elsewhere (e.g. the code editor). + const panels = [{ id: 'ses_a', active: false }]; + assert.equal(pickActivePanelId(panels, 'ses_a'), 'ses_a'); + }); + + test('prefers the currently focused panel over the last active one', () => { + const panels = [ + { id: 'ses_a', active: false }, + { id: 'ses_b', active: true }, + ]; + assert.equal(pickActivePanelId(panels, 'ses_a'), 'ses_b'); + }); + + test('uses the focused panel even when there is no recorded last active panel', () => { + assert.equal(pickActivePanelId([{ id: 'ses_b', active: true }], null), 'ses_b'); + }); + + test('returns the last active panel when no panel is focused', () => { + const panels = [ + { id: 'ses_a', active: false }, + { id: 'ses_b', active: false }, + ]; + assert.equal(pickActivePanelId(panels, 'ses_b'), 'ses_b'); + }); + + test('returns null when nothing is focused and there is no recent panel', () => { + assert.equal(pickActivePanelId([{ id: 'ses_a', active: false }], null), null); + }); +}); diff --git a/packages/vscode/src/activePanelRouting.ts b/packages/vscode/src/activePanelRouting.ts new file mode 100644 index 00000000..e63752db --- /dev/null +++ b/packages/vscode/src/activePanelRouting.ts @@ -0,0 +1,17 @@ +/** + * Pure selection logic shared by the session editor panel routing + * (`*ToActivePanel` methods). Kept free of the `vscode` dependency so it can be + * unit tested in isolation. + * + * A right-click command targets the panel the user is currently in. We prefer a + * panel that is actively focused; otherwise we fall back to the panel that was + * focused most recently. The caller is responsible for confirming the returned + * id still maps to a live panel. + */ +export function pickActivePanelId( + panels: Array<{ id: string; active: boolean }>, + lastActivePanelId: string | null, +): string | null { + const active = panels.find((panel) => panel.active); + return active?.id ?? lastActivePanelId ?? null; +} diff --git a/packages/vscode/src/extension.ts b/packages/vscode/src/extension.ts index 652796be..3fe00e7b 100644 --- a/packages/vscode/src/extension.ts +++ b/packages/vscode/src/extension.ts @@ -6,8 +6,21 @@ import { createOpenCodeManager, type OpenCodeManager } from './opencode'; import { startGlobalEventWatcher, stopGlobalEventWatcher, setChatViewProvider } from './sessionActivityWatcher'; import { pathsEqualWithNormalizedDriveLetter } from './pathUtils'; import { resolveWorkspaceFolders } from './workspaceResolver'; +import { InlineCommentThreads, SIDEBAR_SURFACE_ID } from './InlineCommentThreads'; let chatViewProvider: ChatViewProvider | undefined; + +/** The webview's `{ drafts: [{ id, text }] }` snapshot, or null when it is not one. */ +function readDraftSnapshot(snapshot: unknown): Array<{ id: string; text: string }> | null { + if (typeof snapshot !== 'object' || snapshot === null || !('drafts' in snapshot) || !Array.isArray(snapshot.drafts)) return null; + const drafts: Array<{ id: string; text: string }> = []; + for (const entry of snapshot.drafts) { + if (typeof entry !== 'object' || entry === null || !('id' in entry) || typeof entry.id !== 'string') continue; + const text = 'text' in entry && typeof entry.text === 'string' ? entry.text : ''; + drafts.push({ id: entry.id, text }); + } + return drafts; +} let agentManagerProvider: AgentManagerPanelProvider | undefined; let sessionEditorProvider: SessionEditorPanelProvider | undefined; let openCodeManager: OpenCodeManager | undefined; @@ -471,6 +484,97 @@ export async function activate(context: vscode.ExtensionContext) { }) ); + // Comments are written where the code is: the thread opens on the selected + // lines and stays there until the message is sent. The composer chips remain + // the authoritative list, so the threads follow what the webview reports. + const inlineCommentThreads = new InlineCommentThreads({ + submitDraft: async (payload) => { + // A comment is written against code the user is reading, so it cannot + // require them to have opened a chat first: with no session tab open, + // one is opened, exactly as the toolbar's new-session button does. + const panelId = sessionEditorProvider?.openWithLineComment(payload, activeSessionId); + if (panelId) { + return panelId; + } + // No session editor at all (provider gone): fall back to the sidebar + // rather than accepting a comment that has nowhere to land. + if (!(await revealChatViewForPayload())) { + return null; + } + if (!chatViewProvider) { + vscode.window.showWarningMessage(t('OpenChamber: Chat sidebar is not ready')); + return null; + } + chatViewProvider.addLineComment(payload); + return SIDEBAR_SURFACE_ID; + }, + removeDraft: (draftId) => { + // Every surface is told, because each webview holds its own draft store + // and only the one actually holding the draft can drop it. Removal is + // idempotent everywhere else. + sessionEditorProvider?.removeLineComment(draftId); + chatViewProvider?.removeLineComment(draftId); + }, + reportUndelivered: () => { + vscode.window.showWarningMessage(t('OpenChamber [Add Comment]: The comment never reached the chat and was discarded')); + }, + avatar: vscode.Uri.joinPath(context.extensionUri, 'assets', 'app-icon.png'), + strings: { + threadLabel: ({ startLine, endLine }) => (startLine === endLine + ? t('Comment on line {0}', String(startLine)) + : t('Comment on lines {0}-{1}', String(startLine), String(endLine))), + author: t('OpenChamber'), + notSent: t('Not sent yet'), + }, + }); + context.subscriptions.push(inlineCommentThreads); + + context.subscriptions.push( + vscode.commands.registerCommand('openchamber.addLineComment', () => { + const editor = vscode.window.activeTextEditor; + if (!editor) { + vscode.window.showWarningMessage(t('OpenChamber [Add Comment]: No active editor')); + return; + } + // Same rule the gutter `+` follows, so the two entry points cannot + // disagree about where a comment is allowed. + if (!inlineCommentThreads.canCommentOn(editor.document.uri)) { + vscode.window.showWarningMessage(t('OpenChamber [Add Comment]: File is outside the workspace')); + return; + } + inlineCommentThreads.openThread(editor.document.uri, editor.selection); + }) + ); + + // Invoked by the thread's own Comment button, and by the gutter `+` flow, + // which both arrive as a CommentReply carrying the typed text. + context.subscriptions.push( + vscode.commands.registerCommand('openchamber.submitLineComment', async (reply: vscode.CommentReply) => { + await inlineCommentThreads.submitReply(reply); + }) + ); + + context.subscriptions.push( + vscode.commands.registerCommand('openchamber.removeLineComment', (thread: vscode.CommentThread) => { + inlineCommentThreads.removeThread(thread); + }) + ); + + // The webview reports its whole draft list whenever it changes; the threads + // follow it. Not contributed in package.json: internal wiring, not a command + // a user should find in the palette. + context.subscriptions.push( + vscode.commands.registerCommand('openchamber.internal.inlineCommentsSync', (message: { snapshot: unknown; surfaceId: string }) => { + // The snapshot crossed the webview boundary as JSON; the surface id was + // stamped by the provider that received it, so an untagged snapshot + // cannot be attributed and is ignored rather than applied to threads it + // may know nothing about. + const drafts = readDraftSnapshot(message.snapshot); + if (!drafts || !message.surfaceId) return; + inlineCommentThreads.reconcile(message.surfaceId, drafts); + }) + ); + context.subscriptions.push( vscode.commands.registerCommand('openchamber.newSession', async (directory?: unknown) => { const candidates = resolveWorkspaceFolders(vscode.workspace.workspaceFolders ?? []); diff --git a/packages/vscode/src/inlineCommentSelection.test.ts b/packages/vscode/src/inlineCommentSelection.test.ts new file mode 100644 index 00000000..5cf76417 --- /dev/null +++ b/packages/vscode/src/inlineCommentSelection.test.ts @@ -0,0 +1,258 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { DELIVERY_CONFIRMATION_TIMEOUT_MS, broadcastRemoval, canCommentOnDocument, drainPending, dropPendingById, nextDraftId, reconcileThreadFate, resolveCommentFilePath, resolveCommentOrigin, selectionLineRange, shouldAbandonUnconfirmed, shouldDisposeOnEmptyBody, snapshotOwnsThread } from './inlineCommentSelection'; + +const selection = (startLine: number, startChar: number, endLine: number, endChar: number) => ({ + start: { line: startLine, character: startChar }, + end: { line: endLine, character: endChar }, +}); + +describe('selectionLineRange', () => { + test('a caret with no selection covers its own line', () => { + assert.deepEqual(selectionLineRange(selection(0, 4, 0, 4)), { startLine: 1, endLine: 1 }); + }); + + test('a partial selection on one line covers that line', () => { + assert.deepEqual(selectionLineRange(selection(11, 2, 11, 30)), { startLine: 12, endLine: 12 }); + }); + + test('a multi-line selection covers every line it touches', () => { + assert.deepEqual(selectionLineRange(selection(4, 0, 7, 12)), { startLine: 5, endLine: 8 }); + }); + + test('stopping at the start of the next line does not count that line', () => { + // Dragging past the end of line 12 lands at (12, 0) but shows nothing + // there, so the comment is on line 12 alone. + assert.deepEqual(selectionLineRange(selection(11, 0, 12, 0)), { startLine: 12, endLine: 12 }); + }); + + test('a selection ending at column 0 of its own line is still that line', () => { + assert.deepEqual(selectionLineRange(selection(3, 0, 3, 0)), { startLine: 4, endLine: 4 }); + }); +}); + +describe('nextDraftId', () => { + test('matches the shared store id format', () => { + assert.match(nextDraftId(1735689600000, 0.123456789), /^icd-1735689600000-[a-z0-9]+$/); + }); + + test('different randomness yields different ids at the same instant', () => { + assert.notEqual(nextDraftId(1, 0.5), nextDraftId(1, 0.9)); + }); +}); + +describe('resolveCommentFilePath', () => { + test('an ordinary file keeps its path', () => { + assert.equal(resolveCommentFilePath('/repo/src/app.ts', ''), '/repo/src/app.ts'); + }); + + test("a Source Control diff resolves to the query's real path", () => { + // The original side of a diff is a `git:` document; its path is not a + // file on disk, but the query names the file it came from. + const query = JSON.stringify({ path: '/repo/src/app.ts', ref: '~' }); + assert.equal(resolveCommentFilePath('/repo/src/app.ts.git', query), '/repo/src/app.ts'); + }); + + test('a malformed query falls back to the URI path', () => { + assert.equal(resolveCommentFilePath('/repo/src/app.ts', 'not json'), '/repo/src/app.ts'); + }); + + test('a query without a usable path falls back to the URI path', () => { + assert.equal(resolveCommentFilePath('/repo/src/app.ts', JSON.stringify({ ref: '~' })), '/repo/src/app.ts'); + assert.equal(resolveCommentFilePath('/repo/src/app.ts', JSON.stringify({ path: ' ' })), '/repo/src/app.ts'); + }); +}); + +describe('resolveCommentOrigin', () => { + const diff = { + original: 'git:/repo/src/app.ts?ref=HEAD', + modified: 'file:///repo/src/app.ts', + }; + + test('identifies both sides of the active diff', () => { + assert.deepEqual(resolveCommentOrigin(diff.original, 'git', diff), { source: 'diff', side: 'original' }); + assert.deepEqual(resolveCommentOrigin(diff.modified, 'file', diff), { source: 'diff', side: 'modified' }); + }); + + test('uses the URI scheme when the active tab is unavailable', () => { + assert.deepEqual(resolveCommentOrigin(diff.original, 'git'), { source: 'diff', side: 'original' }); + assert.deepEqual(resolveCommentOrigin(diff.modified, 'file'), { source: 'file' }); + }); +}); + +describe('canCommentOnDocument', () => { + test('a workspace file can take a comment', () => { + assert.equal(canCommentOnDocument('file', true), true); + }); + + test("a diff's original side can too, since it resolves to a workspace file", () => { + assert.equal(canCommentOnDocument('git', true), true); + }); + + test('a file outside the workspace cannot', () => { + // The comment is filed against a workspace-relative path, so one written + // elsewhere would name a file the composer cannot resolve. + assert.equal(canCommentOnDocument('file', false), false); + }); + + test('a comment editor cannot comment on itself', () => { + assert.equal(canCommentOnDocument('comment', true), false); + }); +}); + +describe('drainPending', () => { + test('every held comment is returned, in order', () => { + // A second comment can be written while a panel is still booting. + // Keeping only the newest silently dropped the first after its thread + // had already reported success. + const pending = ['first', 'second', 'third']; + assert.deepEqual(drainPending(pending), ['first', 'second', 'third']); + }); + + test('the hold is emptied, so a later flush delivers nothing twice', () => { + const pending = ['only']; + drainPending(pending); + assert.deepEqual(pending, []); + assert.deepEqual(drainPending(pending), []); + }); + + test('an empty hold drains to nothing', () => { + assert.deepEqual(drainPending([]), []); + }); +}); + +describe('dropPendingById', () => { + const held = () => [{ draftId: 'a' }, { draftId: 'b' }, { draftId: 'c' }]; + + test('a comment removed before it was ever delivered is dropped from the hold', () => { + // Removing the thread while the panel is still booting used to leave the + // payload queued, so the draft landed after the user had dropped it. + const pending = held(); + assert.equal(dropPendingById(pending, 'b'), true); + assert.deepEqual(pending.map((p) => p.draftId), ['a', 'c']); + }); + + test('an id that is not held leaves the queue untouched', () => { + const pending = held(); + assert.equal(dropPendingById(pending, 'zzz'), false); + assert.deepEqual(pending.map((p) => p.draftId), ['a', 'b', 'c']); + }); + + test('an empty hold reports nothing dropped', () => { + assert.equal(dropPendingById([], 'a'), false); + }); +}); + +describe('shouldAbandonUnconfirmed', () => { + test('a comment the composer never reported holding is given up on', () => { + // The panel's webview never booted, or the message was dropped. The + // thread would otherwise show "Not sent yet" forever for a comment that + // cannot be sent and cannot be rewritten. + assert.equal(shouldAbandonUnconfirmed(undefined), true); + assert.equal(shouldAbandonUnconfirmed(false), true); + }); + + test('a comment the composer confirmed holding is kept', () => { + assert.equal(shouldAbandonUnconfirmed(true), false); + }); + + test('the deadline outlasts the composer own wait for a directory', () => { + // The webview waits up to 10s for a directory before filing the draft, + // so a shorter deadline here would abandon comments that were fine. + assert.ok(DELIVERY_CONFIRMATION_TIMEOUT_MS > 10_000); + }); +}); + +describe('broadcastRemoval', () => { + const surface = (draftIds: string[]) => { + const notified: number[] = []; + return { + pendingLineComments: draftIds.map((draftId) => ({ draftId })), + notify: () => notified.push(1), + notified, + }; + }; + + test('every surface is told, because only one of them holds the draft', () => { + const a = surface([]); + const b = surface([]); + broadcastRemoval([a, b], 'icd-1'); + assert.equal(a.notified.length, 1); + assert.equal(b.notified.length, 1); + }); + + test('a comment still held for a booting surface is dropped from the hold', () => { + // The notification alone would find nothing: an undelivered comment is + // in no store yet, and would land after the user removed its thread. + const holding = surface(['icd-1', 'icd-2']); + broadcastRemoval([holding], 'icd-1'); + assert.deepEqual(holding.pendingLineComments.map((p) => p.draftId), ['icd-2']); + }); + + test('surfaces holding nothing keep their queues intact', () => { + const other = surface(['icd-9']); + broadcastRemoval([other], 'icd-1'); + assert.deepEqual(other.pendingLineComments.map((p) => p.draftId), ['icd-9']); + }); + + test('no surfaces at all is not an error', () => { + assert.doesNotThrow(() => broadcastRemoval([], 'icd-1')); + }); +}); + +describe('snapshotOwnsThread', () => { + test('the surface holding the draft speaks for its thread', () => { + assert.equal(snapshotOwnsThread('panel-a', 'panel-a'), true); + }); + + test('another tab says nothing about this thread', () => { + // Every webview has its own draft store, so a second session tab + // reporting an empty list is not evidence that this comment is gone. + // Before this rule, opening a tab disposed the other tab's threads. + assert.equal(snapshotOwnsThread('panel-a', 'panel-b'), false); + }); + + test('the sidebar does not speak for a panel, nor a panel for the sidebar', () => { + assert.equal(snapshotOwnsThread('panel-a', 'sidebar'), false); + assert.equal(snapshotOwnsThread('sidebar', 'panel-a'), false); + }); + + test('a thread with no surface yet is owned by nobody', () => { + assert.equal(snapshotOwnsThread(undefined, 'panel-a'), false); + assert.equal(snapshotOwnsThread('', 'panel-a'), false); + }); +}); + +describe('reconcileThreadFate', () => { + test('a draft absent from the very first snapshot is still in flight', () => { + // Opening a session tab makes its webview publish before the comment + // that opened it has landed. Treating that as a removal destroyed the + // thread the user had just written. + assert.equal(reconcileThreadFate(undefined, false), 'wait'); + }); + + test('a draft absent after having been seen was removed', () => { + assert.equal(reconcileThreadFate(undefined, true), 'dispose'); + }); + + test('a draft present is shown, and counts as seen', () => { + assert.equal(reconcileThreadFate('fix this', false), 'show'); + assert.equal(reconcileThreadFate('fix this', true), 'show'); + }); + + test('a draft emptied in the composer drops its thread', () => { + assert.equal(reconcileThreadFate('', true), 'dispose'); + assert.equal(reconcileThreadFate(' ', false), 'dispose'); + }); +}); + +describe('shouldDisposeOnEmptyBody', () => { + test('blank and whitespace-only bodies are a cancel', () => { + assert.equal(shouldDisposeOnEmptyBody(''), true); + assert.equal(shouldDisposeOnEmptyBody(' \n\t '), true); + }); + + test('any real text is kept', () => { + assert.equal(shouldDisposeOnEmptyBody(' fix this '), false); + }); +}); diff --git a/packages/vscode/src/inlineCommentSelection.ts b/packages/vscode/src/inlineCommentSelection.ts new file mode 100644 index 00000000..23e548c4 --- /dev/null +++ b/packages/vscode/src/inlineCommentSelection.ts @@ -0,0 +1,199 @@ +/** + * Pure selection and identity logic for editor comment threads. Kept free of + * the `vscode` dependency so it can be unit tested in isolation. + */ + +export interface LineRange { + startLine: number; + endLine: number; +} + +interface SelectionLike { + start: { line: number; character: number }; + end: { line: number; character: number }; +} + +/** + * The 1-based inclusive line range a selection covers, as a reader sees it. + * + * Dragging to the start of the next line selects a trailing newline but shows + * nothing on that line, so counting it would label a one-line comment as two + * and send a range that does not match the highlight. + */ +export function selectionLineRange(selection: SelectionLike): LineRange { + const startLine = selection.start.line + 1; + const spansLines = selection.end.line > selection.start.line; + const stopsAtLineStart = selection.end.character === 0 && spansLines; + const endLine = (stopsAtLineStart ? selection.end.line - 1 : selection.end.line) + 1; + return { startLine, endLine }; +} + +/** + * A draft id in the shared store's format. + * + * The extension mints it so the thread and the composer chip agree on identity + * without a round trip; the store accepts a caller-provided id for exactly this. + */ +export function nextDraftId(now: number, randomFraction: number): string { + return `icd-${now}-${randomFraction.toString(36).substring(2, 9)}`; +} + +/** An empty body is a cancel, not a comment worth keeping on screen. */ +export function shouldDisposeOnEmptyBody(body: string): boolean { + return body.trim().length === 0; +} + +/** + * The real file a comment target refers to. + * + * A diff opened from Source Control shows one pane per side, and the original + * side is not a file on disk: it is a `git:` document carrying the real path in + * its JSON query. Labelling a comment with the raw URI path would name a file + * the composer cannot match, so the query wins when it has one. + * + * @param path the URI path (already query-free, as `fsPath` gives it) + * @param query the URI query, empty for ordinary files + */ +export function resolveCommentFilePath(path: string, query: string): string { + if (!query) return path; + try { + const parsed: { path?: string } = JSON.parse(query); + return parsed.path?.trim() ? parsed.path : path; + } catch { + return path; + } +} + +export type CommentOrigin = { + source: 'diff' | 'file'; + side?: 'original' | 'modified'; +}; + +/** Preserves which side of an active diff supplied the selected code. */ +export function resolveCommentOrigin( + uri: string, + scheme: string, + activeDiff?: { original: string; modified: string }, +): CommentOrigin { + if (activeDiff?.original === uri) return { source: 'diff', side: 'original' }; + if (activeDiff?.modified === uri) return { source: 'diff', side: 'modified' }; + if (scheme === 'git') return { source: 'diff', side: 'original' }; + return { source: 'file' }; +} + +/** + * Whether a document can take a comment. + * + * Both entry points ask this, so the gutter `+` and the right-click command + * agree about where commenting is allowed. A comment is filed against a + * workspace-relative path, so one written outside the workspace would name a + * file the composer cannot resolve. + */ +export function canCommentOnDocument(scheme: string, isInWorkspace: boolean): boolean { + if (scheme === 'comment') return false; + return isInWorkspace; +} + +/** + * Empties a hold of comments waiting on a webview that had not booted. + * + * A hold is a list, not a single slot: a user can write a second comment while + * the panel is still starting, and keeping only the newest silently dropped the + * first after its thread had already reported success. + */ +export function drainPending(pending: T[]): T[] { + return pending.splice(0, pending.length); +} + +/** + * Removes a held comment the user dropped before it was ever delivered. + * + * A comment waiting on a booting webview is in no store yet, so asking that + * webview to remove it finds nothing. Without dropping the hold too, the draft + * would land after the user had already removed its thread. + * + * @returns whether a held comment was dropped + */ +export function dropPendingById(pending: T[], draftId: string): boolean { + const index = pending.findIndex((entry) => entry.draftId === draftId); + if (index < 0) return false; + pending.splice(index, 1); + return true; +} + +/** + * How long a submitted comment may go unconfirmed before it is given up on. + * + * Long enough to outlast a cold webview boot plus the composer's own wait for a + * directory, short enough that a thread does not sit there promising to send + * something that never will. + */ +export const DELIVERY_CONFIRMATION_TIMEOUT_MS = 30_000; + +/** + * Whether a submitted comment should be abandoned once its deadline passes. + * + * Confirmation means the composer reported holding the draft. Without it the + * comment reached no store: the panel's webview never booted, or the message + * was dropped. Keeping the thread would show "Not sent yet" forever, for a + * comment that cannot be sent and cannot be rewritten — only deleted. + */ +export function shouldAbandonUnconfirmed(confirmed: boolean | undefined): boolean { + return !confirmed; +} + +/** A chat surface that may be holding, or showing, a comment draft. */ +export interface RemovalTarget { + /** Comments still waiting on this surface's webview to boot. */ + pendingLineComments: Array<{ draftId?: string }>; + /** Asks this surface's webview to drop the draft from its store. */ + notify: () => void; +} + +/** + * Tells every surface to drop a comment, wherever it currently lives. + * + * Each webview owns its own draft store, so the one holding the draft cannot be + * known from here; every surface is told and the rest no-op. The hold is cleared + * before notifying, because a comment that has not been delivered yet is in no + * store for the notification to find, and would otherwise arrive afterwards as a + * chip the user had already dropped. + */ +export function broadcastRemoval(targets: Iterable, draftId: string): void { + for (const target of targets) { + dropPendingById(target.pendingLineComments, draftId); + target.notify(); + } +} + +/** + * Whether a draft snapshot is authoritative for a thread. + * + * Every webview — the sidebar and each session tab — runs its own draft store + * and publishes its whole list. Only the surface that accepted a comment knows + * whether it still holds it; to any other surface the draft simply never + * existed. Letting a foreign snapshot decide disposed threads that were alive + * and about to be sent, which is what opening a second tab used to do. + */ +export function snapshotOwnsThread(threadSurfaceId: string | undefined, snapshotSurfaceId: string): boolean { + return Boolean(threadSurfaceId) && threadSurfaceId === snapshotSurfaceId; +} + +/** What a draft-list snapshot says should happen to one editor thread. */ +export type ThreadFate = 'wait' | 'dispose' | 'show'; + +/** + * Decides a thread's fate from the composer's current draft list. + * + * `confirmed` records whether the composer has ever reported holding this + * draft. Until it has, absence means the delivery is still in flight, not that + * the comment was removed: opening a session tab produces a first snapshot + * describing the composer as it was before the comment that opened it arrived. + * + * @param text the draft's text in the snapshot, or undefined when absent + */ +export function reconcileThreadFate(text: string | undefined, confirmed: boolean): ThreadFate { + if (text === undefined) return confirmed ? 'dispose' : 'wait'; + if (shouldDisposeOnEmptyBody(text)) return 'dispose'; + return 'show'; +} diff --git a/packages/vscode/webview/api/bridge.ts b/packages/vscode/webview/api/bridge.ts index 48c10f95..c14f7517 100644 --- a/packages/vscode/webview/api/bridge.ts +++ b/packages/vscode/webview/api/bridge.ts @@ -84,6 +84,18 @@ export function sendBridgeMessage(type: string, payload?: unknown): return sendBridgeMessageWithOptions(type, payload); } +/** + * Tells the extension something without waiting for an answer. + * + * Requests are tracked until a response arrives, so a message the extension + * never replies to would leak a pending entry on every call. State the webview + * pushes outward (editor comment threads following the composer's drafts) has + * no answer to wait for, so it does not go through the request path at all. + */ +export function postBridgeNotification(type: string, payload: Payload): void { + getVSCodeAPI().postMessage({ type, payload }); +} + export function sendBridgeMessageWithOptions( type: string, payload?: unknown, diff --git a/packages/vscode/webview/inlineCommentRemovals.test.ts b/packages/vscode/webview/inlineCommentRemovals.test.ts new file mode 100644 index 00000000..12aaaf7a --- /dev/null +++ b/packages/vscode/webview/inlineCommentRemovals.test.ts @@ -0,0 +1,60 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; +import { createRemovalTombstones } from './inlineCommentRemovals'; + +describe('inline comment removal tombstones', () => { + test('a delivery arriving after its removal is dropped', () => { + // The window this closes: the extension holds a payload for a booting + // panel, or the handler waits for a directory, and the user removes the + // thread meanwhile. Without this the draft lands as a chip they dropped. + const tombstones = createRemovalTombstones(); + tombstones.remember('icd-1'); + assert.equal(tombstones.consume('icd-1'), true); + }); + + test('an unrelated delivery is untouched', () => { + const tombstones = createRemovalTombstones(); + tombstones.remember('icd-1'); + assert.equal(tombstones.consume('icd-2'), false); + }); + + test('a delivery with no id is never dropped', () => { + // Comments from other entry points carry no draft id. + const tombstones = createRemovalTombstones(); + tombstones.remember('icd-1'); + assert.equal(tombstones.consume(undefined), false); + }); + + test('the record is consumed, so only the delayed delivery is refused', () => { + const tombstones = createRemovalTombstones(); + tombstones.remember('icd-1'); + tombstones.consume('icd-1'); + assert.equal(tombstones.consume('icd-1'), false); + assert.equal(tombstones.size(), 0); + }); + + test('remembering the same removal twice keeps one record', () => { + const tombstones = createRemovalTombstones(); + tombstones.remember('icd-1'); + tombstones.remember('icd-1'); + assert.equal(tombstones.size(), 1); + }); + + test('an empty id is not recorded', () => { + const tombstones = createRemovalTombstones(); + tombstones.remember(''); + assert.equal(tombstones.size(), 0); + }); + + test('the record is bounded, evicting the oldest first', () => { + const tombstones = createRemovalTombstones(3); + for (const id of ['a', 'b', 'c', 'd']) tombstones.remember(id); + + assert.equal(tombstones.size(), 3); + // 'a' aged out; the three most recent still refuse their deliveries. + assert.equal(tombstones.consume('a'), false); + assert.equal(tombstones.consume('d'), true); + assert.equal(tombstones.consume('c'), true); + assert.equal(tombstones.consume('b'), true); + }); +}); diff --git a/packages/vscode/webview/inlineCommentRemovals.ts b/packages/vscode/webview/inlineCommentRemovals.ts new file mode 100644 index 00000000..34531207 --- /dev/null +++ b/packages/vscode/webview/inlineCommentRemovals.ts @@ -0,0 +1,49 @@ +/** + * Comments the user dropped before their draft reached this webview's store. + * + * Delivery is asynchronous on both sides: the extension holds a payload for a + * panel that has not booted, and the handler that files the draft can wait + * seconds for a directory to resolve. A removal can arrive anywhere in that + * window, when there is no draft yet to remove. Recording it here lets the + * delayed delivery recognise a comment that is no longer wanted, instead of + * filing it as a chip the user already dropped and sending it with the next + * message. + * + * Bounded because it is a tombstone list, not state: ids are unique per comment, + * so entries are never revisited once their delivery window has passed. + */ + +const REMEMBERED_REMOVALS = 50; + +export function createRemovalTombstones(limit: number = REMEMBERED_REMOVALS) { + const ids = new Set(); + + return { + /** Records a removal, evicting the oldest once the bound is reached. */ + remember(draftId: string): void { + if (!draftId) return; + ids.add(draftId); + if (ids.size > limit) { + const oldest = ids.values().next(); + if (!oldest.done) ids.delete(oldest.value); + } + }, + + /** + * Whether this delivery should be dropped. + * + * Consumes the record: the window closes once the delayed delivery has + * been refused, and a later comment reusing the id would be unrelated. + */ + consume(draftId: string | undefined): boolean { + if (!draftId || !ids.has(draftId)) return false; + ids.delete(draftId); + return true; + }, + + /** Number of removals currently remembered. Exposed for tests. */ + size(): number { + return ids.size; + }, + }; +} diff --git a/packages/vscode/webview/main.tsx b/packages/vscode/webview/main.tsx index 4a4bc409..feb8cd14 100644 --- a/packages/vscode/webview/main.tsx +++ b/packages/vscode/webview/main.tsx @@ -1,5 +1,6 @@ import { createVSCodeAPIs } from './api'; -import { onCommand, onThemeChange, proxyApiRequest, proxySessionMessageRequest, sendBridgeMessage, startSseProxy, stopSseProxy } from './api/bridge'; +import { createRemovalTombstones } from './inlineCommentRemovals'; +import { onCommand, onThemeChange, postBridgeNotification, proxyApiRequest, proxySessionMessageRequest, sendBridgeMessage, startSseProxy, stopSseProxy } from './api/bridge'; import { vscodeStreamPerfCount, vscodeStreamPerfMeasure, vscodeStreamPerfObserve } from './api/streamPerf'; import { extractBodyBase64, extractBodyText, extractJsonBody, hasInitBody } from './requestBodyTransport'; import type { RuntimeAPIs } from '@openchamber/ui/lib/api/types'; @@ -1329,6 +1330,159 @@ onCommand('addContextSelection', (payload) => { }); }); +// Comments dropped from their editor thread before the draft reached this +// store. See the module for why the window exists. +const removedComments = createRemovalTombstones(); + +onCommand('addLineComment', (payload) => { + // SAFETY: the payload crossed the extension boundary as JSON; every field is + // read as unknown here and trusted only after the checks below. + const record = payload as { + draftId?: unknown; + filePath?: unknown; + relativePath?: unknown; + source?: unknown; + side?: unknown; + startLine?: unknown; + endLine?: unknown; + code?: unknown; + language?: unknown; + comment?: unknown; + }; + + // The editor thread mints the id so it can track its own draft without a + // round trip. Absent when the comment came from anywhere else. + const draftId = typeof record.draftId === 'string' && record.draftId ? record.draftId : undefined; + const relativePath = typeof record.relativePath === 'string' ? record.relativePath : ''; + const source = record.source === 'diff' ? 'diff' : 'file'; + const side = record.side === 'original' || record.side === 'modified' ? record.side : undefined; + const startLine = typeof record.startLine === 'number' ? record.startLine : 1; + const endLine = typeof record.endLine === 'number' ? record.endLine : startLine; + const code = typeof record.code === 'string' ? record.code : ''; + const language = typeof record.language === 'string' ? record.language : 'text'; + const comment = typeof record.comment === 'string' ? record.comment.trim() : ''; + + if (!relativePath) { + console.warn('[openchamber] inline comment arrived without a path; dropping', record); + return; + } + + void Promise.all([ + import('@/sync/session-ui-store'), + import('@/stores/useDirectoryStore'), + import('@/stores/useInlineCommentDraftStore'), + ]).then(async ([{ useSessionUIStore }, { useDirectoryStore }, { useInlineCommentDraftStore }]) => { + // Inline drafts are owned by runtime + directory + session. Both halves are + // read together, from one store snapshot: read apart, a session that + // finished loading between them would pair its key with the previous + // session's directory, and the draft would land under a key ChatInput never + // reads. Directory precedence matches the composer's own. + const resolveTarget = () => { + const sessionState = useSessionUIStore.getState(); + const currentSessionId = sessionState.currentSessionId; + const sessionDirectory = currentSessionId ? sessionState.getDirectoryForSession(currentSessionId) : null; + const draftDirectory = sessionState.newSessionDraft?.open + ? sessionState.newSessionDraft.bootstrapPendingDirectory ?? sessionState.newSessionDraft.directoryOverride ?? null + : null; + const directory = sessionDirectory ?? draftDirectory ?? useDirectoryStore.getState().currentDirectory; + return directory ? { directory, sessionKey: currentSessionId ?? 'draft' } : null; + }; + + // A comment can arrive before the chat surface has finished booting: the + // extension opens the sidebar and posts after a fixed delay, which a cold + // webview can outlast. Dropping the draft here loses a comment the user + // already wrote and already saw accepted in the editor, so wait for the + // target to land instead. + let target = resolveTarget(); + for (let attempt = 0; !target && attempt < 40; attempt += 1) { + await new Promise((resolve) => setTimeout(resolve, 250)); + target = resolveTarget(); + } + if (!target) { + console.warn('[openchamber] no directory resolved; dropping inline comment', { relativePath, startLine }); + return; + } + + // Checked after the wait, which is the window the removal can land in. + if (removedComments.consume(draftId)) { + return; + } + + useInlineCommentDraftStore.getState().addDraft(target, { + id: draftId, + source, + fileLabel: relativePath, + startLine, + endLine, + side, + code, + language, + text: comment, + }); + }); +}); + +// The editor's comment threads mirror the composer's drafts, so every change to +// the draft store is reported as a whole snapshot. Sending the full list rather +// than add/remove events means a dropped notification cannot leave a thread +// anchored to a comment that is no longer attached; sending the message empties +// the list, which clears the threads through the same path. +void import('@/stores/useInlineCommentDraftStore').then(({ useInlineCommentDraftStore }) => { + let lastSignature = ''; + + const publish = (drafts: Record>) => { + const flat = Object.values(drafts) + .flat() + .map((draft) => ({ id: draft.id, text: draft.text })); + const signature = JSON.stringify(flat); + if (signature === lastSignature) return; + lastSignature = signature; + postBridgeNotification('inlineComments:sync', { drafts: flat }); + }; + + publish(useInlineCommentDraftStore.getState().drafts); + useInlineCommentDraftStore.subscribe((state) => publish(state.drafts)); +}); + +onCommand('removeLineComment', (payload) => { + if (typeof payload !== 'object' || payload === null || !('draftId' in payload)) return; + const { draftId } = payload; + if (typeof draftId !== 'string' || !draftId) { + return; + } + + // Recorded even when the draft is already here: the store removal below is + // the normal path, and this only matters when the draft has not landed yet. + removedComments.remember(draftId); + + void Promise.all([ + import('@/stores/useInlineCommentDraftStore'), + import('@/lib/runtime-switch'), + ]).then(([{ useInlineCommentDraftStore }, { getRuntimeKey }]) => { + const state = useInlineCommentDraftStore.getState(); + const runtimeKey = getRuntimeKey(); + + // The thread knows its draft id but not which target holds it. Search for + // the owning key, and only within the current runtime: `removeDraft` + // recomputes the key from the live runtime, so a target rebuilt from + // another runtime's key would delete from the wrong place. + for (const [key, drafts] of Object.entries(state.drafts)) { + if (!drafts.some((draft) => draft.id === draftId)) continue; + let parsed: unknown; + try { + parsed = JSON.parse(key); + } catch { + continue; + } + if (!Array.isArray(parsed) || parsed.length !== 3 || !parsed.every((segment) => typeof segment === 'string')) continue; + const [keyRuntime, directory, sessionKey] = parsed; + if (keyRuntime !== runtimeKey) continue; + state.removeDraft({ directory, sessionKey }, draftId); + return; + } + }); +}); + onCommand('addFileMentions', (payload) => { const rawPaths = Array.isArray((payload as { paths?: unknown[] })?.paths) ? (payload as { paths: unknown[] }).paths