diff --git a/package.json b/package.json index 936c6e3..1e3cfd7 100644 --- a/package.json +++ b/package.json @@ -107,6 +107,10 @@ "command": "opencode.insertFileReference", "title": "OpenCode: Insert File Reference" }, + { + "command": "opencode.addToThread", + "title": "OpenCode: Add to Thread" + }, { "command": "opencode.installSkill", "title": "OpenCode: Install Skill" @@ -152,6 +156,16 @@ "command": "opencode.sendSelection", "when": "editorHasSelection", "group": "opencode" + }, + { + "command": "opencode.addToThread", + "group": "opencode" + } + ], + "explorer/context": [ + { + "command": "opencode.addToThread", + "group": "opencode" } ] }, diff --git a/src/extension.ts b/src/extension.ts index cc81090..154bfb1 100644 --- a/src/extension.ts +++ b/src/extension.ts @@ -425,6 +425,99 @@ export async function activate(context: vscode.ExtensionContext) { ), ); + // ============================================================================ + // COMMAND: opencode.addToThread + // ============================================================================ + // Purpose: Add editor selection, whole file, or explorer file(s)/folder(s) + // to the active chat thread as context. + // Invocation contexts: + // - Editor right-click (with or without selection) + // - Explorer right-click on file(s) or folder(s) + // - Command palette + // Integration: Adds context via ChatViewProvider.addContext() + // ============================================================================ + context.subscriptions.push( + vscode.commands.registerCommand( + "opencode.addToThread", + async (input?: vscode.Uri | vscode.Uri[]) => { + const uris = input + ? Array.isArray(input) + ? input + : [input] + : []; + + if (uris.length === 0) { + const editor = vscode.window.activeTextEditor; + if (!editor) { + vscode.window.showWarningMessage("No active editor or file selected"); + return; + } + const file = vscode.workspace.asRelativePath(editor.document.uri); + const selection = editor.document.getText(editor.selection); + if (selection && selection.trim().length > 0) { + const startLine = editor.selection.start.line + 1; + const endLine = editor.selection.end.line + 1; + const lineInfo = + startLine === endLine ? `${startLine}` : `${startLine}-${endLine}`; + await chatViewProvider.addContext({ + file, + lineInfo, + content: selection, + languageId: editor.document.languageId, + }); + } else { + await chatViewProvider.addContext({ + file, + languageId: editor.document.languageId, + }); + } + await vscode.commands.executeCommand("opencode.chatView.focus"); + return; + } + + const MAX_FILES = 50; + const collectedUris: vscode.Uri[] = []; + for (const uri of uris) { + try { + const stat = await vscode.workspace.fs.stat(uri); + if (stat.type === vscode.FileType.Directory) { + const relPattern = new vscode.RelativePattern(uri, "**/*"); + const found = await vscode.workspace.findFiles( + relPattern, + "**/{node_modules,.git,dist,build,out,.next,.cache}/**", + MAX_FILES - collectedUris.length, + ); + collectedUris.push(...found); + } else { + collectedUris.push(uri); + } + } catch { + collectedUris.push(uri); + } + if (collectedUris.length >= MAX_FILES) break; + } + + if (collectedUris.length === 0) { + vscode.window.showWarningMessage("No files found to add"); + return; + } + + if (collectedUris.length > MAX_FILES) { + vscode.window.showWarningMessage( + `Too many files (${collectedUris.length}). Added first ${MAX_FILES}.`, + ); + collectedUris.length = MAX_FILES; + } + + for (const fileUri of collectedUris) { + const file = vscode.workspace.asRelativePath(fileUri); + await chatViewProvider.addContext({ file }); + } + await vscode.commands.executeCommand("opencode.chatView.focus"); + }, + ), + ); + // ============================================================================ // COMMAND: opencode.showPlan // ============================================================================ diff --git a/src/providers/ChatViewProvider.ts b/src/providers/ChatViewProvider.ts index c662e0a..b69db03 100644 --- a/src/providers/ChatViewProvider.ts +++ b/src/providers/ChatViewProvider.ts @@ -1682,6 +1682,10 @@ export class ChatViewProvider } }); + // Sync persisted revert state so the webview knows if this session + // is currently reverted (Undo button → Restore after reload). + await this.syncRevertStateFromServer(sessionId); + // Update the list selection await this.handleGetSessions(); } catch (error) { @@ -3158,6 +3162,9 @@ export class ChatViewProvider await promptAsyncFn.call((client as any).session, { sessionID: replySessionId, directory: this.getWorkspaceDirectory(), + // Include the agent so the server uses the correct agent instead of + // falling back to the workspace default (which may not exist). + agent: this.modelAndAgentManager.getSelectedAgent(), // Include parts so the agent knows what the user answered parts: [{ type: "text", text: displayText }], }); @@ -3354,6 +3361,12 @@ export class ChatViewProvider ); break; } + case "unrevertSession": { + await this.handleUnrevertSession( + this.firstNonEmptyString(message.sessionId), + ); + break; + } case "getMessageFileDiffPreview": { await this.handleGetMessageFileDiffPreview( this.firstNonEmptyString(message.messageId), @@ -5018,7 +5031,7 @@ export class ChatViewProvider promptFiles.push({ uri: path.isAbsolute(filePath) ? vscode.Uri.file(filePath).toString() : filePath, mime: "text/plain", - name: path.basename(filePath), + name: filePath, }); } for (const context of options.contexts ?? []) { @@ -5027,20 +5040,44 @@ export class ChatViewProvider continue; } const content = typeof context?.content === "string" ? context.content : ""; - promptFiles.push({ - uri: filePath, - mime: typeof context?.languageId === "string" ? context.languageId : "text/plain", - name: path.basename(filePath), - ...(content - ? { - source: { - start: 0, - end: content.length, - text: content, - }, - } - : {}), - }); + if (content) { + // Code selection: use data URI so the server only sees the selected lines, + // not the whole file from disk. Without this, the server resolves the raw + // file path and the AI reads the entire file instead of the selection. + const dataUri = `data:text/plain;base64,${Buffer.from( + content, + "utf-8", + ).toString("base64")}`; + const lineInfo = + typeof context?.lineInfo === "string" ? context.lineInfo : ""; + const nameWithLine = + filePath && lineInfo ? `${filePath}:${lineInfo}` : filePath; + promptFiles.push({ + uri: dataUri, + mime: "text/plain", + name: nameWithLine, + source: { + type: "file", + path: filePath, + text: { + value: content, + start: 0, + end: content.length, + }, + lineInfo, + languageId: + typeof context?.languageId === "string" ? context.languageId : "", + }, + }); + } else { + promptFiles.push({ + uri: path.isAbsolute(filePath) + ? vscode.Uri.file(filePath).toString() + : filePath, + mime: "text/plain", + name: filePath, + }); + } } for (const image of options.images ?? []) { const dataUrl = @@ -7064,9 +7101,32 @@ export class ChatViewProvider } as any, }); } else if (ctx.content) { + const selectionContent = ctx.content; + const selectionPath = ctx.file; + const selectionDataUrl = `data:text/plain;base64,${Buffer.from( + selectionContent, + "utf-8", + ).toString("base64")}`; + const selectionPathWithLineInfo = + selectionPath && ctx.lineInfo + ? `${selectionPath}:${ctx.lineInfo}` + : selectionPath; parts.push({ - type: "text", - text: `\`\`\`${ctx.languageId}\n// ${ctx.file}:${ctx.lineInfo}\n${ctx.content}\n\`\`\``, + type: "file", + mime: "text/plain", + filename: selectionPathWithLineInfo, + url: selectionDataUrl, + source: { + type: "file", + path: selectionPath || "", + text: { + value: selectionContent, + start: 0, + end: selectionContent.length, + }, + lineInfo: ctx.lineInfo || "", + languageId: ctx.languageId || "", + } as any, }); } else if (ctx.file && workspaceFolder) { // Handle file paths without content (attached via @) @@ -7081,7 +7141,7 @@ export class ChatViewProvider const textContent = new TextDecoder().decode(content); parts.push({ type: "file", - mime: ctx.languageId || "text/plain", + mime: "text/plain", filename: ctx.file.split(/[\\/]/).pop(), url: absoluteUri.toString(), source: { @@ -9430,18 +9490,80 @@ export class ChatViewProvider requestedSessionId, this.currentSessionId, ); + // Surface the early-return case so the user knows why nothing happened + // instead of silently failing. if (!targetMessageId || !targetSessionId) { + vscode.window.showWarningMessage( + "Unable to undo changes: missing message or session identifier.", + ); return; } try { const client = await this.serverManager.ensureRunning(); const workspaceDir = this.getWorkspaceDirectory(); - await client.session.revert({ + // The v2 SDK defaults to ThrowOnError=false, which means HTTP errors + // (400 BadRequest, 404 NotFound, 409 SessionBusy) are returned in + // result.error — NOT thrown. We must check result.error explicitly + // before accessing result.data, otherwise the undo silently fails. + const revertResult = await client.session.revert({ sessionID: targetSessionId, messageID: targetMessageId, ...(workspaceDir ? { directory: workspaceDir } : {}), }); + const revertError = ( + revertResult as unknown as { error?: unknown } + )?.error; + if (revertError) { + const errorMessage = + revertError instanceof Error + ? revertError.message + : typeof revertError === "object" && + revertError !== null + ? String( + (revertError as Record).message ?? + (revertError as Record).data ?? + revertError, + ) + : String(revertError); + this.logger.error("Undo changes: server returned error", { + messageId: targetMessageId, + sessionId: targetSessionId, + error: errorMessage, + }); + vscode.window.showErrorMessage( + `Failed to undo changes: ${errorMessage}`, + ); + return; + } + + const sessionData = ( + revertResult as unknown as { data?: unknown } + )?.data; + const revertField = + (sessionData as Record | undefined)?.revert ?? + undefined; + const revertRecord = + revertField && typeof revertField === "object" + ? (revertField as Record) + : null; + const optionalStr = (key: string): string | undefined => { + const v = revertRecord ? revertRecord[key] : undefined; + return typeof v === "string" && v.length > 0 ? v : undefined; + }; + const revertState = revertRecord + ? { + messageID: + optionalStr("messageID") ?? targetMessageId, + partID: optionalStr("partID"), + snapshot: optionalStr("snapshot"), + diff: optionalStr("diff"), + } + : null; + this.view?.webview.postMessage({ + type: "revertStateUpdate", + revertState, + }); await this.handleLoadSession(targetSessionId); await this.handleGetSessions(); @@ -9457,6 +9579,104 @@ export class ChatViewProvider } } + private async handleUnrevertSession( + requestedSessionId?: string, + ): Promise { + const targetSessionId = this.firstNonEmptyString( + requestedSessionId, + this.currentSessionId, + ); + if (!targetSessionId) { + vscode.window.showWarningMessage( + "Unable to restore: missing session identifier.", + ); + return; + } + + try { + const client = await this.serverManager.ensureRunning(); + const workspaceDir = this.getWorkspaceDirectory(); + // Same ThrowOnError=false pattern as handleUndoMessageChanges — + // HTTP errors land in result.error, not in catch. + const unrevertResult = await client.session.unrevert({ + sessionID: targetSessionId, + ...(workspaceDir ? { directory: workspaceDir } : {}), + }); + + const unrevertError = ( + unrevertResult as unknown as { error?: unknown } + )?.error; + if (unrevertError) { + const errorMessage = + unrevertError instanceof Error + ? unrevertError.message + : typeof unrevertError === "object" && + unrevertError !== null + ? String( + (unrevertError as Record).message ?? + (unrevertError as Record).data ?? + unrevertError, + ) + : String(unrevertError); + this.logger.error("Restore: server returned error", { + sessionId: targetSessionId, + error: errorMessage, + }); + vscode.window.showErrorMessage( + `Failed to restore: ${errorMessage}`, + ); + return; + } + + this.view?.webview.postMessage({ + type: "revertStateUpdate", + revertState: null, + }); + + await this.handleLoadSession(targetSessionId); + await this.handleGetSessions(); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + this.logger.error("Failed to restore reverted messages", { + sessionId: targetSessionId, + error: errorMessage, + }); + vscode.window.showErrorMessage(`Failed to restore: ${errorMessage}`); + } + } + + private async syncRevertStateFromServer(sessionId: string): Promise { + try { + const client = await this.serverManager.ensureRunning(); + const resp = await client.session.get({ sessionID: sessionId }); + const revert = (resp.data as Record | undefined)?.revert as + | { messageID?: string; partID?: string; snapshot?: string; diff?: string } + | undefined; + if (revert?.messageID) { + this.view?.webview.postMessage({ + type: "revertStateUpdate", + revertState: { + messageID: revert.messageID, + partID: revert.partID, + snapshot: revert.snapshot, + diff: revert.diff, + }, + }); + } else { + this.view?.webview.postMessage({ + type: "revertStateUpdate", + revertState: null, + }); + } + } catch (error) { + this.logger.debug("syncRevertStateFromServer: could not fetch session", { + sessionId, + error: error instanceof Error ? error.message : String(error), + }); + } + } + private async handleGetMessageFileDiffPreview( messageId?: string, filePath?: string, diff --git a/src/shared/centralizedDebugPayloadFilter.ts b/src/shared/centralizedDebugPayloadFilter.ts index b359351..a60092e 100644 --- a/src/shared/centralizedDebugPayloadFilter.ts +++ b/src/shared/centralizedDebugPayloadFilter.ts @@ -19,6 +19,8 @@ const CENTRALIZED_DEBUG_EXCLUDED_PATH_RULES = [ "message.part.delta", "message.part.delta.1", "tui.toast.show", + "tui.show", + "session.status", ], }, { @@ -30,6 +32,8 @@ const CENTRALIZED_DEBUG_EXCLUDED_PATH_RULES = [ "message.part.delta", "message.part.delta.1", "tui.toast.show", + "tui.show", + "session.status", ], }, { @@ -41,6 +45,34 @@ const CENTRALIZED_DEBUG_EXCLUDED_PATH_RULES = [ "message.part.delta", "message.part.delta.1", "tui.toast.show", + "tui.show", + "session.status", + ], + }, + { + path: "syncEvent.data.type", + values: [ + "server.connected", + "server.connected.1", + "server.heartbeat", + "message.part.delta", + "message.part.delta.1", + "tui.toast.show", + "tui.show", + "session.status", + ], + }, + { + path: "payload.syncEvent.data.type", + values: [ + "server.connected", + "server.connected.1", + "server.heartbeat", + "message.part.delta", + "message.part.delta.1", + "tui.toast.show", + "tui.show", + "session.status", ], }, ] as const; @@ -166,17 +198,23 @@ function stripDotPathIfJsonSchema( // This fallback chain is critical for ensuring that live stream events are correctly // classified and not dropped by the centralized data persister. Without this, // stream events will resolve to an empty type and be incorrectly rejected as noise. -function normalizedCentralizedEventType(event: Record): string { +export function normalizedCentralizedEventType(event: Record): string { const directType = asString(event.type ?? event.event ?? event.kind).trim(); const payloadRecord = asRecord(event.payload); const payloadType = asString(payloadRecord?.type ?? payloadRecord?.event ?? payloadRecord?.kind).trim(); - const payloadSyncType = asString(asRecord(payloadRecord?.syncEvent)?.type).trim(); - const syncType = asString(asRecord(event.syncEvent)?.type).trim(); + const syncEvent = asRecord(event.syncEvent); + const syncType = asString(syncEvent?.type).trim(); + const syncData = asRecord(syncEvent?.data); + const syncDataType = asString(syncData?.type ?? syncData?.event ?? syncData?.kind).trim(); + const payloadSyncEvent = asRecord(payloadRecord?.syncEvent); + const payloadSyncType = asString(payloadSyncEvent?.type).trim(); + const payloadSyncData = asRecord(payloadSyncEvent?.data); + const payloadSyncDataType = asString(payloadSyncData?.type ?? payloadSyncData?.event ?? payloadSyncData?.kind).trim(); const rawType = directType && directType !== "sync" ? directType - : payloadSyncType || syncType || payloadType || directType; + : payloadSyncType || syncType || payloadSyncDataType || syncDataType || payloadType || directType; return rawType.replace(/\.\d+$/, ""); } @@ -281,7 +319,10 @@ export function getCentralizedDebugPayloadDisposition( (expected) => asString(value).trim().toLowerCase() === expected.toLowerCase(), ) ) { - return record.type === "tui.toast.show" ? "live-only" : "excluded-noise"; + const normalizedType = normalizedCentralizedEventType(record); + return normalizedType === "tui.toast.show" || normalizedType === "tui.show" || normalizedType === "session.status" + ? "live-only" + : "excluded-noise"; } } } diff --git a/tests/integration/code-selection-flow.test.mjs b/tests/integration/code-selection-flow.test.mjs new file mode 100644 index 0000000..3af12eb --- /dev/null +++ b/tests/integration/code-selection-flow.test.mjs @@ -0,0 +1,558 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { readSource, joinFromRoot } from '../helpers/source-utils.mjs'; + +/** + * Code-Selection Attachment Flow — Integration Tests + * + * Guards the end-to-end flow from editor selection (Ctrl+L / Cmd+L) through + * ChatViewProvider part construction, webview type definitions, and React + * rendering helpers. Every layer must agree that a code selection is a + * structured `{type:"file", source:{type:"file", lineInfo:"31-32", text:{...}}}` part — + * NOT a flat `{type:"text", text:"```lang\n// path:line\ncontent\n```"}`. + * + * Regression triggers: + * - Reverting to flat-text part shape (selection content vanishes from UI) + * - isExplicitFileAttachmentPart matching code_selection (duplicate chips) + * - Missing type guard / collector helpers (chips never render) + * - Strip helpers running on structured parts (content stripped) + */ + +const providerSource = readSource( + [joinFromRoot('src', 'providers', 'ChatViewProvider.ts')], + 'ChatViewProvider.ts', +); + +const messageComponentsSource = readSource( + [joinFromRoot('webview', 'shared', 'src', 'chat', 'MessageComponents.tsx')], + 'MessageComponents.tsx', +); + +const typesSource = readSource( + [joinFromRoot('webview', 'shared', 'src', 'chat', 'lib', 'types.ts')], + 'types.ts', +); + +const modalSource = readSource( + [joinFromRoot('webview', 'shared', 'src', 'chat', 'CodeSelectionPreviewModal.tsx')], + 'CodeSelectionPreviewModal.tsx', +); + +// ============================================================================ +// Layer 1: Extension Host — Part Construction +// ============================================================================ + +describe('Code-selection part construction (ChatViewProvider)', () => { + test('code selection context emits structured file part with file source', () => { + // The ctx.content branch must push a {type:"file"} part with source.type:"file" + assert.match( + providerSource, + /else if \(ctx\.content\)\s*\{[\s\S]*?type:\s*"file"[\s\S]*?source:\s*\{[\s\S]*?type:\s*"file"/, + 'ctx.content branch must emit type:"file" with source.type:"file"', + ); + }); + + test('code selection part includes path, text payload, lineInfo, and languageId', () => { + assert.match( + providerSource, + /source:\s*\{[\s\S]*?type:\s*"file"[\s\S]*?path:\s*selectionPath[\s\S]*?text:\s*\{[\s\S]*?value:\s*selectionContent[\s\S]*?start:\s*0[\s\S]*?end:\s*selectionContent\.length[\s\S]*?lineInfo:\s*ctx\.lineInfo[\s\S]*?languageId:\s*ctx\.languageId/, + 'file source must carry path, text{value,start,end}, lineInfo, and languageId', + ); + }); + + test('deferred SDK prompt contexts always use text/plain instead of languageId-derived mime', () => { + assert.match( + providerSource, + /for \(const context of options\.contexts \?\? \[\]\) \{[\s\S]*?promptFiles\.push\(\{[\s\S]*?mime:\s*"text\/plain"/, + 'deferred SDK prompt contexts must no longer derive mime from context.languageId', + ); + assert.doesNotMatch( + providerSource, + /mime:\s*typeof context\?\.languageId === "string" \? context\.languageId : "text\/plain"/, + 'deferred SDK prompt must not send language IDs like markdown/typescript as file MIME types', + ); + }); + + test('deferred SDK prompt files and contexts keep full path metadata instead of basename-only names', () => { + assert.match( + providerSource, + /promptFiles\.push\(\{[\s\S]*?uri:\s*path\.isAbsolute\(filePath\) \? vscode\.Uri\.file\(filePath\)\.toString\(\) : filePath,[\s\S]*?name:\s*filePath/, + 'plain attached files must preserve full filePath in the prompt file name metadata', + ); + // Code selections in the deferred path now use a data URI (not raw file path) + // so the server can't resolve the whole file from disk. + assert.match( + providerSource, + /for \(const context of options\.contexts \?\? \[\]\) \{[\s\S]*?if \(content\) \{[\s\S]*?dataUri[\s\S]*?name:\s*nameWithLine/, + 'deferred code selections must use data URI with nameWithLine for AI metadata', + ); + assert.match( + providerSource, + /const dataUri = `data:text\/plain;base64,\$\{Buffer\.from\([\s\S]*?content[\s\S]*?"base64"\)\}/, + 'deferred path must base64-encode selection content as data URI', + ); + }); + + test('code selection filename preserves full path plus line info for AI metadata while UI can still derive short labels', () => { + assert.match( + providerSource, + /const\s+selectionPathWithLineInfo\s*=\s*[\s\S]*?selectionPath && ctx\.lineInfo[\s\S]*?filename:\s*selectionPathWithLineInfo/, + 'ctx.content branch must stamp full selection path plus lineInfo into filename metadata so the AI receives the complete path context', + ); + }); + + test('code selection part must NOT use the old flat-text markdown fence', () => { + // The old bug: text:"```lang\n// file:line\ncontent\n```" + // After our fix, the ctx.content branch should NOT contain the backtick fence pattern + const ctxContentBranch = providerSource.match( + /else if \(ctx\.content\)\s*\{([\s\S]*?)\}\s*else if/, + ); + assert.ok(ctxContentBranch, 'ctx.content branch must exist and be followed by another else-if'); + assert.doesNotMatch( + ctxContentBranch[1], + /text:\s*`\\\u0060\\\u0060\\\u0060\$\{ctx\.languageId\}/, + 'ctx.content branch must NOT push a markdown-fenced text part', + ); + assert.doesNotMatch( + ctxContentBranch[1], + /type:\s*"text"[\s\S]*?\/\/\s*\$\{ctx\.file\}:\$\{ctx\.lineInfo\}/, + 'ctx.content branch must NOT embed // path:line header in a text part', + ); + }); + + test('resource and file-reference branches remain unchanged', () => { + assert.match( + providerSource, + /startsWith\("resource:"\)[\s\S]*?type:\s*"file"[\s\S]*?type:\s*"resource"\s+as const/, + 'resource branch must still emit structured file part with resource source', + ); + assert.match( + providerSource, + /else if \(ctx\.file && workspaceFolder\)[\s\S]*?type:\s*"file"[\s\S]*?type:\s*"file"[\s\S]*?path:\s*ctx\.file/, + 'file-reference branch must still emit structured file part with path source', + ); + }); + + test('code selection url uses data URI with selection content instead of file URI to whole file', () => { + const ctxContentBranch = providerSource.match( + /else if \(ctx\.content\)\s*\{([\s\S]*?)\}\s*else if/, + ); + assert.ok(ctxContentBranch, 'ctx.content branch must exist'); + assert.match( + ctxContentBranch[1], + /selectionDataUrl\s*=\s*`data:text\/plain;base64,[\s\S]*?Buffer\.from\([\s\S]*?selectionContent/, + 'ctx.content branch must build a data:text/plain URI from the selection content, not a file:// URI to the whole file', + ); + assert.match( + ctxContentBranch[1], + /url:\s*selectionDataUrl/, + 'ctx.content branch url field must use the selection data URL', + ); + assert.doesNotMatch( + ctxContentBranch[1], + /vscode\.Uri\.file\(selectionPath\)/, + 'ctx.content branch must NOT construct a file:// URI from the selection path — the server would read the whole file', + ); + }); +}); + +// ============================================================================ +// Layer 2: Webview Type Definitions +// ============================================================================ + +describe('Code-selection type definitions (types.ts)', () => { + test('MessagePart.source is broadened beyond {path?}', () => { + assert.match( + typesSource, + /export\s+interface\s+MessagePartSource\s*\{[\s\S]*?type\?[\s\S]*?languageId\?[\s\S]*?lineInfo\?/, + 'MessagePartSource interface must include type, languageId, and lineInfo optional fields', + ); + assert.match( + typesSource, + /source\?:\s*MessagePartSource/, + 'MessagePart.source must reference MessagePartSource type', + ); + }); + + test('CodeSelectionSource interface is exported with file discriminator', () => { + assert.match( + typesSource, + /export\s+interface\s+CodeSelectionSource\s+extends\s+MessagePartSource/, + 'CodeSelectionSource must extend MessagePartSource', + ); + assert.match( + typesSource, + /CodeSelectionSource[\s\S]*?type:\s*"file"/, + 'CodeSelectionSource must declare type:"file" discriminator', + ); + assert.match( + typesSource, + /CodeSelectionSource[\s\S]*?path:\s*string[\s\S]*?text:\s*\{\s*value:\s*string;\s*start:\s*number;\s*end:\s*number\s*\}/, + 'CodeSelectionSource must carry required path and text{value,start,end} fields', + ); + }); + + test('CodeSelectionMessagePart interface is exported', () => { + assert.match( + typesSource, + /export\s+interface\s+CodeSelectionMessagePart\s+extends\s+MessagePart/, + 'CodeSelectionMessagePart must extend MessagePart', + ); + assert.match( + typesSource, + /CodeSelectionMessagePart[\s\S]*?source:\s*CodeSelectionSource/, + 'CodeSelectionMessagePart must require source: CodeSelectionSource', + ); + }); +}); + +// ============================================================================ +// Layer 3: Webview Helpers — Type Guard + Collector +// ============================================================================ + +describe('Code-selection helpers (MessageComponents.tsx)', () => { + test('isCodeSelectionPart type guard exists with correct discriminator logic', () => { + assert.match( + messageComponentsSource, + /function\s+isCodeSelectionPart\s*\([^)]*\):\s*part is CodeSelectionMessagePart/, + 'isCodeSelectionPart must be a type predicate function returning "part is CodeSelectionMessagePart"', + ); + assert.match( + messageComponentsSource, + /isCodeSelectionPart[\s\S]*?type\s*===\s*"file"\s*&&\s*sourceType\s*===\s*"file"\s*&&\s*lineInfo\.length\s*>\s*0/, + 'isCodeSelectionPart must check type==="file" AND sourceType==="file" AND lineInfo present', + ); + }); + + test('isExplicitFileAttachmentPart EXCLUDES code_selection parts (BUG #1 regression)', () => { + // This is the critical regression guard: code_selection parts have type:"file" + // and must NOT be picked up by isExplicitFileAttachmentPart (which would create + // duplicate chips — one filename-only, one filename:lineRange). + const funcBody = extractFunctionBodyFromTsx( + messageComponentsSource, + 'function isExplicitFileAttachmentPart(', + ); + assert.ok(funcBody, 'isExplicitFileAttachmentPart must exist'); + assert.match( + funcBody, + /partType\s*===\s*"file"[\s\S]*?isCodeSelectionPart\(part\)/, + 'isExplicitFileAttachmentPart must call isCodeSelectionPart before returning true for file parts', + ); + assert.match( + funcBody, + /if\s*\(isCodeSelectionPart\(part\)\)\s*return\s*false/, + 'isExplicitFileAttachmentPart must return false when isCodeSelectionPart is true', + ); + }); + + test('collectCodeSelectionsFromParts extracts chip data from parts array', () => { + assert.match( + messageComponentsSource, + /function\s+collectCodeSelectionsFromParts\s*\([\s\S]*?\):\s*CodeSelectionChipData\[\]/, + 'collectCodeSelectionsFromParts must return CodeSelectionChipData[]', + ); + const body = extractFunctionBodyFromTsx( + messageComponentsSource, + 'function collectCodeSelectionsFromParts(', + ); + assert.match(body, /isCodeSelectionPart\(part\)/, 'must filter via isCodeSelectionPart'); + assert.match(body, /source\.text\?\.value/, 'must extract content from source.text.value'); + assert.match(body, /parseLineRange\(source\.lineInfo\)/, 'must parse line range from source.lineInfo'); + assert.match(body, /filename:\s*basenamePreservingLineSuffix\(part\.filename \|\| source\.path \|\| "", lineInfo\)\s*\|\|\s*undefined/, 'must derive chip filename from full-path metadata through the basename-preserving helper'); + assert.match(body, /languageId:\s*source\.languageId\s*\|\|\s*part\.mime/, 'must fall back to part.mime for languageId'); + }); + + test('parseLineRange handles single line, range, and missing lineInfo', () => { + const body = extractFunctionBodyFromTsx( + messageComponentsSource, + 'function parseLineRange(', + ); + assert.ok(body, 'parseLineRange must exist'); + assert.match(body, /lineInfo\.match\(/, 'must call lineInfo.match with a regex'); + assert.match(body, /match\[1\]\s*\?\s*Number\(match\[1\]\)/, 'must convert match[1] to startLine'); + assert.match(body, /match\[2\]\s*\?\s*Number\(match\[2\]\)/, 'must convert match[2] to endLine'); + assert.match(body, /if\s*\(!lineInfo\)\s*return\s*\{\}/, 'must return empty for missing lineInfo'); + }); + + test('CodeSelectionChipData interface is exported', () => { + assert.match( + messageComponentsSource, + /export\s+interface\s+CodeSelectionChipData\s*\{[\s\S]*?path\?[\s\S]*?filename\?[\s\S]*?languageId\?[\s\S]*?lineInfo\?[\s\S]*?content:\s*string[\s\S]*?startLine\?[\s\S]*?endLine\?/, + 'CodeSelectionChipData must be exported with all chip-rendering fields', + ); + }); +}); + +// ============================================================================ +// Layer 4: UserMessage Rendering +// ============================================================================ + +describe('Code-selection chip rendering (UserMessage component)', () => { + test('UserMessage collects code selections from message parts', () => { + assert.match( + messageComponentsSource, + /collectCodeSelectionsFromParts\(message\?\.parts\)/, + 'UserMessage must call collectCodeSelectionsFromParts on message.parts', + ); + }); + + test('code-selection chips render with FileCode icon and filename:lineRange label', () => { + // Find the chip rendering block — must use FileCode icon and build a label with line range + assert.match( + messageComponentsSource, + /codeSelections\.length\s*>\s*0[\s\S]*? { + assert.match( + messageComponentsSource, + /fileChips\.length\s*>\s*0[\s\S]*? { + assert.match( + messageComponentsSource, + /function\s+buildExplicitFileChipLabel\s*\(/, + 'buildExplicitFileChipLabel helper must exist', + ); + const body = extractFunctionBodyFromTsx( + messageComponentsSource, + 'function buildExplicitFileChipLabel(', + ); + assert.match(messageComponentsSource, /function\s+basenamePreservingLineSuffix\s*\(/, 'MessageComponents must define a helper that collapses full-path metadata into a basename display label'); + assert.match(body, /basenamePreservingLineSuffix\(filename \|\| sourcePath, lineInfo\)/, 'helper must collapse full-path metadata back to a basename for chip display'); + assert.match(body, /part\.source\?\.lineInfo/, 'helper must read source.lineInfo when available'); + assert.match(body, /baseLabel\.endsWith\(lineSuffix\)/, 'helper must avoid duplicating an existing :line suffix'); + assert.match(body, /return\s+baseLabel\.endsWith\(lineSuffix\)\s*\?\s*baseLabel\s*:\s*`\$\{baseLabel\}\$\{lineSuffix\}`/, 'helper must append :lineInfo to the displayed label when needed'); + }); + + test('code selection collector derives a basename label even when filename metadata carries the full path', () => { + assert.match( + messageComponentsSource, + /filename:\s*basenamePreservingLineSuffix\(part\.filename \|\| source\.path \|\| "", lineInfo\)\s*\|\|\s*undefined/, + 'collectCodeSelectionsFromParts must derive short chip labels from full-path filename metadata', + ); + }); + + test('clicking a code-selection chip opens CodeSelectionPreviewModal', () => { + assert.match( + messageComponentsSource, + /setPreviewSelection\(sel\)/, + 'clicking a code-selection chip must set previewSelection state', + ); + assert.match( + messageComponentsSource, + / { + // Both chip types must use the same Tailwind classes for visual consistency + const chipClassPattern = /rounded-full border border-oc-border bg-oc-panel-soft px-2\.5 py-1 text-\[10px\] font-medium text-oc-text-soft transition-colors hover:bg-oc-bg-soft/; + const matches = messageComponentsSource.match(new RegExp(chipClassPattern.source, 'g')); + assert.ok(matches && matches.length >= 2, 'at least 2 chip elements must share the same CSS class string'); + }); + + test('render guards include codeSelections in empty-message and content checks', () => { + // The UserMessage must not bail out early when only code selections are present + assert.match( + messageComponentsSource, + /codeSelections\.length\s*===\s*0/, + 'empty-message guard must include codeSelections.length === 0', + ); + assert.match( + messageComponentsSource, + /codeSelections\.length\s*>\s*0/, + 'content-or-attachments guard must include codeSelections.length > 0', + ); + assert.match( + messageComponentsSource, + /fileChips\.length\s*===\s*0/, + 'empty-message guard must include fileChips.length === 0', + ); + assert.match( + messageComponentsSource, + /fileChips\.length\s*>\s*0/, + 'content-or-attachments guard must include fileChips.length > 0', + ); + }); + + test('user text aggregation excludes synthetic tool/file-dump text parts', () => { + assert.match( + messageComponentsSource, + /function\s+isSyntheticUserToolTextPart\s*\(/, + 'user message rendering must define a helper for synthetic tool/file-dump text parts', + ); + assert.match( + messageComponentsSource, + /\(part as \{ synthetic\?: unknown \}\)\.synthetic === true/, + 'synthetic text parts must be excluded structurally, not only by string heuristics', + ); + assert.match( + messageComponentsSource, + /normalized\.startsWith\("called the "\)[\s\S]*?normalized\.includes\(" tool with the following input:"\)/, + 'synthetic tool-call echo text must be filtered from user bubbles', + ); + assert.match( + messageComponentsSource, + /text\.includes\(""\)[\s\S]*?text\.includes\("<\/path>"\)[\s\S]*?text\.includes\(""\)/, + 'raw file-dump fragments must be filtered from user bubbles', + ); + assert.match( + messageComponentsSource, + /role\?\.toLowerCase\(\) === "user"[\s\S]*?isRenderableUserTextPart/, + 'messageBodyFromParts must use stricter filtering for user messages', + ); + }); +}); + +// ============================================================================ +// Layer 5: Stripping Short-Circuit +// ============================================================================ + +describe('Stripping short-circuit for structured code selections', () => { + test('stripHydratedAttachmentEcho and stripGenericHydratedAttachmentFence are bypassed when code_selection parts exist', () => { + // When structured code_selection parts are present, the markdown stripping + // helpers must be skipped (the structured data is authoritative). + assert.match( + messageComponentsSource, + /codeSelections\.length\s*>\s*0[\s\S]*?(skip|bypass|short-circuit|return|without)/i, + 'codeSelections presence must trigger a skip/bypass of stripping helpers', + ); + }); + + test('legacy strip helpers still exist for old persisted messages', () => { + // Old messages without code_selection parts still need strip + infer path + assert.match( + messageComponentsSource, + /function\s+stripHydratedAttachmentEcho/, + 'stripHydratedAttachmentEcho must still exist for legacy messages', + ); + assert.match( + messageComponentsSource, + /function\s+stripGenericHydratedAttachmentFence/, + 'stripGenericHydratedAttachmentFence must still exist for legacy messages', + ); + assert.match( + messageComponentsSource, + /function\s+inferAttachmentPathsFromHydratedUserText/, + 'inferAttachmentPathsFromHydratedUserText must still exist for legacy messages', + ); + }); +}); + +// ============================================================================ +// Layer 6: Preview Modal +// ============================================================================ + +describe('CodeSelectionPreviewModal component', () => { + test('modal component is exported and uses createPortal', () => { + assert.match(modalSource, /export\s+function\s+CodeSelectionPreviewModal/, 'must be exported'); + assert.match(modalSource, /createPortal/, 'must use createPortal for DOM-level rendering'); + }); + + test('modal supports escape-to-close and backdrop click', () => { + assert.match(modalSource, /Escape/, 'must handle Escape key'); + assert.match(modalSource, /onClose/, 'must call onClose'); + assert.match(modalSource, /aria-modal/, 'must set aria-modal attribute'); + }); + + test('modal uses highlight.js for syntax highlighting with auto fallback', () => { + assert.match(modalSource, /hljs\.highlight\(code,\s*\{\s*language:\s*lang\s*\}\)/, 'must use hljs.highlight with language'); + assert.match(modalSource, /hljs\.highlightAuto\(code\)/, 'must fall back to hljs.highlightAuto'); + assert.match(modalSource, /hljs\.getLanguage\(lang\)/, 'must check language availability before highlighting'); + }); + + test('modal header shows filename:lineRange title and language badge', () => { + assert.match(modalSource, /buildTitle/, 'must have a title builder'); + assert.match(modalSource, /buildLineLabel/, 'must have a line label builder'); + assert.match(modalSource, /languageBadge|data\.languageId/i, 'must render language badge'); + assert.match(modalSource, /start\s*&&\s*end\s*&&\s*start\s*!==\s*end/, 'line label builder must handle line ranges'); + }); +}); + +// ============================================================================ +// Helper: Extract function body from TSX (same logic as extractFunctionBody +// but used inline to avoid import ambiguity for TSX files) +// ============================================================================ + +function extractFunctionBodyFromTsx(source, signature) { + const idx = source.indexOf(signature); + if (idx === -1) return ''; + const after = source.slice(idx + signature.length); + // Find the first top-level { ... } block (could be a return type annotation + // like `: { foo?: bar }` or the function body itself). + let depth = 0; + let startIdx = -1; + let firstBlockEnd = -1; + for (let i = 0; i < after.length; i++) { + const ch = after[i]; + if (ch === '{') { + if (depth === 0) startIdx = i + 1; + depth++; + } else if (ch === '}') { + depth--; + if (depth === 0 && startIdx !== -1) { + firstBlockEnd = i; + break; + } + } + } + if (firstBlockEnd === -1) return ''; + // Check if the next non-whitespace character after the first block's `}` + // is another `{`. If so, the first block was a return type annotation and + // the second block is the real function body. + const rest = after.slice(firstBlockEnd + 1); + const nextNonWs = rest.match(/^\s*(\S)/); + if (nextNonWs && nextNonWs[1] === '{') { + // Extract the second { ... } block (the actual body). + let d = 0; + let si = -1; + for (let i = 0; i < rest.length; i++) { + const ch = rest[i]; + if (ch === '{') { + if (d === 0) si = i + 1; + d++; + } else if (ch === '}') { + d--; + if (d === 0 && si !== -1) { + return rest.slice(si, i); + } + } + } + return ''; + } + // No second block — the first block IS the function body. + return after.slice(startIdx, firstBlockEnd); +} diff --git a/tests/integration/duplicate-bubble-reconciliation.test.mjs b/tests/integration/duplicate-bubble-reconciliation.test.mjs new file mode 100644 index 0000000..69afef7 --- /dev/null +++ b/tests/integration/duplicate-bubble-reconciliation.test.mjs @@ -0,0 +1,118 @@ +/** + * Regression: Synthetic user text parts must not create duplicate + * optimistic user bubbles in the centralized transcript. + * + * Bug: When a user message contained real text ("read this line") plus + * synthetic text parts (tool call echoes like "Called the Read tool..." + * and file dump content), the centralized builder used text-pattern + * heuristics to filter synthetic content. Some synthetic parts (like + * markdown headers "## Phase 2...") didn't match the heuristic, so the + * canonical visible text became "read this line\n\n## Phase 2...". + * + * The pending optimistic overlay only knew "read this line", so + * normalizeComparableText comparison failed and the optimistic bubble + * survived alongside the canonical message → duplicate "read this line" + * bubbles. + * + * Fix: buildVisibleUserMessageText() now structurally filters parts with + * `synthetic !== true` BEFORE extracting text, so all synthetic parts are + * excluded regardless of content patterns. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readSource, joinFromRoot } from "../helpers/source-utils.mjs"; + +const chatShellSource = readSource( + [joinFromRoot("webview", "shared", "src", "chat", "ChatShell.tsx")], + "ChatShell.tsx", +); + +const messageComponentsSource = readSource( + [joinFromRoot("webview", "shared", "src", "chat", "MessageComponents.tsx")], + "MessageComponents.tsx", +); + +function extractBuildVisibleUserMessageText(source) { + const start = source.indexOf("function buildVisibleUserMessageText("); + if (start === -1) return ""; + const end = source.indexOf("\n}\n", start); + return source.slice(start, end + 2); +} + +test("buildVisibleUserMessageText structurally filters synthetic parts", () => { + const body = extractBuildVisibleUserMessageText(chatShellSource); + assert.ok(body.length > 0, "function must exist"); + + assert.match( + body, + /\.filter\(\(part\) => part\?\.synthetic !== true\)/, + "must filter out parts with synthetic: true BEFORE extracting text", + ); + + assert.match( + body, + /splitInjectedSystemPromptFromUserText/, + "must still split injected system prompts", + ); + + assert.match( + body, + /isSyntheticUserToolText/, + "must still apply text-pattern filtering as secondary defense", + ); + + assert.match( + body, + /normalizeComparableText/, + "must dedupe visible texts by normalized comparison", + ); +}); + +test("buildVisibleUserMessageText falls back to rawText when all parts are synthetic", () => { + const body = extractBuildVisibleUserMessageText(chatShellSource); + + assert.match( + body, + /if \(visibleTexts\.length === 0\)/, + "must check for empty visible texts after filtering", + ); + + assert.match( + body, + /return splitInjectedSystemPromptFromUserText\(rawText/, + "must fall back to rawText when no visible parts remain", + ); +}); + +test("isRenderableUserTextPart structurally excludes synthetic parts for user role", () => { + const fnStart = messageComponentsSource.indexOf("function isRenderableUserTextPart("); + assert.ok(fnStart > -1, "isRenderableUserTextPart must exist"); + + const fnBody = messageComponentsSource.slice(fnStart, fnStart + 600); + + assert.match( + fnBody, + /\(part as \{ synthetic\?: unknown \}\)\.synthetic === true/, + "must check synthetic flag and return false for synthetic parts", + ); +}); + +test("messageBodyFromParts accepts optional role parameter for user-aware filtering", () => { + const fnStart = messageComponentsSource.indexOf("function messageBodyFromParts("); + assert.ok(fnStart > -1, "messageBodyFromParts must exist"); + + const fnBody = messageComponentsSource.slice(fnStart, fnStart + 800); + + assert.match( + fnBody, + /role\??: string/, + "must accept optional role parameter", + ); + + assert.match( + fnBody, + /role\?\.toLowerCase\(\) === "user"/, + "must check for user role to apply stricter filtering", + ); +}); diff --git a/tests/providers/add-to-thread-command.test.mjs b/tests/providers/add-to-thread-command.test.mjs new file mode 100644 index 0000000..5df8499 --- /dev/null +++ b/tests/providers/add-to-thread-command.test.mjs @@ -0,0 +1,277 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { readSource, joinFromRoot } from '../helpers/source-utils.mjs'; + +/** + * "Add to OpenCode Thread" Context Menu — Integration Tests + * + * Guards the context-menu feature that lets users add editor selections, whole + * files, or explorer file/folder selections to the active chat thread. + * Parity with Codex's "Add to Thread" behavior. + * + * Regression triggers: + * - Command missing from package.json contributes + * - Menu items missing from editor/context or explorer/context + * - Handler not covering folder recursion + * - MAX_FILES cap removed (unbounded file expansion) + */ + +const extensionSource = readSource( + [joinFromRoot('src', 'extension.ts')], + 'extension.ts', +); + +const packageSource = readSource( + [joinFromRoot('package.json')], + 'package.json', +); + +// ============================================================================ +// Layer 1: Command Registration (package.json) +// ============================================================================ + +describe('package.json command contribution', () => { + test('opencode.addToThread command is registered', () => { + assert.match( + packageSource, + /"command":\s*"opencode\.addToThread"/, + 'opencode.addToThread must be in contributes.commands', + ); + assert.match( + packageSource, + /"title":\s*"OpenCode: Add to Thread"/, + 'command title must be "OpenCode: Add to Thread"', + ); + }); +}); + +// ============================================================================ +// Layer 2: Menu Contributions (package.json) +// ============================================================================ + +describe('package.json menu contributions', () => { + test('editor/context includes addToThread', () => { + // The editor/context menu must include addToThread so users can right-click + // in the editor (with or without a selection) to add content to the thread. + const editorContextMatch = packageSource.match( + /"editor\/context":\s*\[([\s\S]*?)\]/, + ); + assert.ok(editorContextMatch, 'editor/context menu must exist in contributes.menus'); + assert.match( + editorContextMatch[1], + /"command":\s*"opencode\.addToThread"/, + 'editor/context must include opencode.addToThread command', + ); + assert.match( + editorContextMatch[1], + /"group":\s*"opencode"/, + 'addToThread must be in the "opencode" group', + ); + }); + + test('explorer/context includes addToThread', () => { + // The explorer/context menu must include addToThread so users can right-click + // files or folders in the file explorer to add them to the thread. + const explorerContextMatch = packageSource.match( + /"explorer\/context":\s*\[([\s\S]*?)\]/, + ); + assert.ok(explorerContextMatch, 'explorer/context menu must exist in contributes.menus'); + assert.match( + explorerContextMatch[1], + /"command":\s*"opencode\.addToThread"/, + 'explorer/context must include opencode.addToThread command', + ); + }); + + test('existing sendSelection editor/context entry is preserved', () => { + const editorContextMatch = packageSource.match( + /"editor\/context":\s*\[([\s\S]*?)\]/, + ); + assert.ok(editorContextMatch); + assert.match( + editorContextMatch[1], + /"command":\s*"opencode\.sendSelection"/, + 'sendSelection must still be in editor/context (Ctrl+L/Cmd+L flow)', + ); + assert.match( + editorContextMatch[1], + /"when":\s*"editorHasSelection"/, + 'sendSelection must still gate on editorHasSelection', + ); + }); +}); + +// ============================================================================ +// Layer 3: Handler Implementation (extension.ts) +// ============================================================================ + +describe('addToThread handler registration', () => { + test('command is registered via registerCommand', () => { + assert.match( + extensionSource, + /registerCommand\(\s*"opencode\.addToThread"/, + 'opencode.addToThread must be registered via vscode.commands.registerCommand', + ); + }); + + test('handler accepts optional Uri or Uri[] argument (explorer multi-select)', () => { + assert.match( + extensionSource, + /"opencode\.addToThread",\s*async\s*\(input\?:\s*vscode\.Uri\s*\|\s*vscode\.Uri\[\]\)\s*=>/, + 'handler must accept optional Uri | Uri[] for explorer context', + ); + }); +}); + +describe('addToThread editor path (no Uri argument)', () => { + test('uses activeTextEditor when no Uri argument is provided', () => { + assert.match( + extensionSource, + /uris\.length\s*===\s*0[\s\S]*?vscode\.window\.activeTextEditor/, + 'handler must fall back to activeTextEditor when no explorer Uri is provided', + ); + }); + + test('with selection: adds context with file, lineInfo, content, languageId', () => { + // When the editor has a selection, the handler must extract: + // file (relative path), lineInfo (e.g. "12-18"), content (selected text), languageId + assert.match( + extensionSource, + /selection\.trim\(\)\.length\s*>\s*0[\s\S]*?chatViewProvider\.addContext\(\s*\{[\s\S]*?file,[\s\S]*?lineInfo,[\s\S]*?content:\s*selection,[\s\S]*?languageId/, + 'editor selection path must call addContext with file, lineInfo, content, languageId', + ); + }); + + test('lineInfo is computed from editor.selection start/end lines', () => { + assert.match( + extensionSource, + /startLine\s*=\s*editor\.selection\.start\.line\s*\+\s*1/, + 'startLine must be 1-indexed (editor.selection.start.line + 1)', + ); + assert.match( + extensionSource, + /endLine\s*=\s*editor\.selection\.end\.line\s*\+\s*1/, + 'endLine must be 1-indexed', + ); + assert.match( + extensionSource, + /startLine\s*===\s*endLine\s*\?\s*`\$\{startLine\}`\s*:\s*`\$\{startLine\}-\$\{endLine\}`/, + 'lineInfo must be single number for single-line, range for multi-line', + ); + }); + + test('without selection: adds whole-file context (no content field)', () => { + // When there's no selection, the handler adds a file reference without content + // — the ChatViewProvider reads the file from disk. + assert.match( + extensionSource, + /else\s*\{[\s\S]*?chatViewProvider\.addContext\(\s*\{[\s\S]*?file,[\s\S]*?languageId[^}]*\}\s*\)/, + 'no-selection path must call addContext with file and languageId only', + ); + }); +}); + +describe('addToThread explorer path (Uri argument)', () => { + test('normalizes single Uri to array', () => { + assert.match( + extensionSource, + /Array\.isArray\(input\)\s*\?\s*input\s*:\s*\[input\]/, + 'handler must normalize single Uri to array for uniform processing', + ); + }); + + test('detects directories via fs.stat and recurses with findFiles', () => { + assert.match( + extensionSource, + /vscode\.workspace\.fs\.stat\(uri\)/, + 'handler must stat each Uri to check if directory', + ); + assert.match( + extensionSource, + /stat\.type\s*===\s*vscode\.FileType\.Directory/, + 'handler must check FileType.Directory', + ); + assert.match( + extensionSource, + /vscode\.workspace\.findFiles\(/, + 'handler must use findFiles for folder recursion', + ); + assert.match( + extensionSource, + /RelativePattern\(/, + 'handler must use RelativePattern for scoped file search', + ); + }); + + test('excludes common non-source directories from folder recursion', () => { + // node_modules, .git, dist, build, out, .next, .cache must be excluded + assert.match( + extensionSource, + /node_modules/, + 'must exclude node_modules', + ); + assert.match( + extensionSource, + /\.git/, + 'must exclude .git', + ); + assert.match( + extensionSource, + /dist|build|out/, + 'must exclude build output directories', + ); + }); + + test('enforces MAX_FILES cap to prevent unbounded expansion', () => { + assert.match( + extensionSource, + /MAX_FILES\s*=\s*\d+/, + 'handler must define a MAX_FILES constant', + ); + assert.match( + extensionSource, + /collectedUris\.length\s*>=\s*MAX_FILES/, + 'handler must break when MAX_FILES is reached', + ); + assert.match( + extensionSource, + /collectedUris\.length\s*>\s*MAX_FILES[\s\S]*?showWarningMessage/, + 'handler must warn the user when MAX_FILES is exceeded', + ); + assert.match( + extensionSource, + /collectedUris\.length\s*=\s*MAX_FILES/, + 'handler must truncate to MAX_FILES', + ); + }); + + test('uses workspace-relative paths when adding file context', () => { + assert.match( + extensionSource, + /vscode\.workspace\.asRelativePath\(fileUri\)/, + 'handler must convert absolute Uris to workspace-relative paths', + ); + assert.match( + extensionSource, + /chatViewProvider\.addContext\(\s*\{\s*file\s*\}\s*\)/, + 'explorer file path must call addContext with relative file path', + ); + }); + + test('focuses chat view after adding context', () => { + assert.match( + extensionSource, + /executeCommand\(\s*"opencode\.chatView\.focus"\s*\)/, + 'handler must focus the chat view after adding context (both editor and explorer paths)', + ); + }); + + test('handles stat failures gracefully (falls back to adding the Uri)', () => { + assert.match( + extensionSource, + /catch\s*\{[\s\S]*?collectedUris\.push\(uri\)/, + 'handler must push the Uri directly if fs.stat fails', + ); + }); +}); diff --git a/tests/providers/question-reply-agent-and-custom-answer.test.mjs b/tests/providers/question-reply-agent-and-custom-answer.test.mjs new file mode 100644 index 0000000..3458ce7 --- /dev/null +++ b/tests/providers/question-reply-agent-and-custom-answer.test.mjs @@ -0,0 +1,119 @@ +/** + * Regression: questionReply promptAsync must include agent, and custom answer + * field must not default to visible when the server doesn't allow it. + * + * Bug A: The promptAsync fallback in the questionReply handler omitted the + * `agent` parameter. The server fell back to the workspace-configured default + * agent, which may not exist, producing: + * Error: default agent "Sisyphus - Ultraworker" not found + * + * Bug B: `interactiveEventsFromQuestionAskedPayload` defaulted + * `asBoolean(question.custom, true)`, making the "Custom Answer..." button + * visible for every question even when the server sent concrete options and + * did not enable custom input. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +const repoRoot = process.cwd(); + +function readSource(relPath) { + return readFileSync(join(repoRoot, relPath), "utf-8"); +} + +function extractFunctionBody(source, signaturePattern) { + const startIdx = source.indexOf(signaturePattern); + if (startIdx === -1) return null; + const braceStart = source.indexOf("{", startIdx); + if (braceStart === -1) return null; + let depth = 0; + let end = braceStart; + for (let i = braceStart; i < source.length; i++) { + const ch = source[i]; + if (ch === "{") depth++; + else if (ch === "}") { + depth--; + if (depth === 0) { + end = i; + break; + } + } + } + return source.slice(braceStart + 1, end); +} + +// --------------------------------------------------------------------------- +// FIX A: questionReply promptAsync must include agent parameter +// --------------------------------------------------------------------------- + +test("questionReply handler — promptAsync fallback includes agent parameter", () => { + const source = readSource("src/providers/ChatViewProvider.ts"); + + // Locate the promptAsync call inside the stale-question fallback path. + const promptAsyncIdx = source.indexOf("promptAsyncFn.call("); + assert.notEqual(promptAsyncIdx, -1, "promptAsyncFn.call should exist in ChatViewProvider"); + + // Grab a generous window around the call to inspect its arguments. + const window = source.slice(promptAsyncIdx, promptAsyncIdx + 600); + + assert.match( + window, + /agent:\s*this\.modelAndAgentManager\.getSelectedAgent\(\)/, + "promptAsync call must include agent: this.modelAndAgentManager.getSelectedAgent()", + ); + assert.match(window, /sessionID:\s*replySessionId/, "promptAsync call must include sessionID"); + assert.match(window, /parts:\s*\[/, "promptAsync call must include parts"); +}); + +// --------------------------------------------------------------------------- +// FIX B: interactiveEventsFromQuestionAskedPayload must not default custom to true +// --------------------------------------------------------------------------- + +test("interactiveEventsFromQuestionAskedPayload defaults question.custom to false, not true", () => { + const source = readSource("webview/shared/src/chat/lib/messageHandler.ts"); + + const body = extractFunctionBody( + source, + "function interactiveEventsFromQuestionAskedPayload(", + ); + assert.ok(body, "interactiveEventsFromQuestionAskedPayload should exist"); + + // The allowCustomInput derivation inside this function must NOT use `true` + // as the default for question.custom. + const allowCustomMatch = body.match( + /allowCustomInput\s*=\s*([\s\S]*?)(?:;\s*\n|;\s*$)/, + ); + assert.ok(allowCustomMatch, "allowCustomInput derivation should exist in function body"); + + const derivation = allowCustomMatch[1]; + assert.match(derivation, /allowCustomInput.*false/, "allowCustomInput should default from allowCustomInput: false"); + assert.match(derivation, /allow_custom_input.*false/, "allowCustomInput should default from allow_custom_input: false"); + + // The critical regression assertion: question.custom must default to false. + assert.doesNotMatch( + derivation, + /question\.custom,\s*true/, + "question.custom must NOT default to true — this causes the custom answer button to appear for every question", + ); + assert.match( + derivation, + /question\.custom,\s*false/, + "question.custom must default to false so custom answer only appears when the server explicitly enables it", + ); +}); + +test("interactiveEventsFromQuestionAskedPayload still allows custom input when server explicitly sets it", () => { + const source = readSource("webview/shared/src/chat/lib/messageHandler.ts"); + const body = extractFunctionBody( + source, + "function interactiveEventsFromQuestionAskedPayload(", + ); + assert.ok(body); + + // Ensure the function still respects explicit allowCustomInput from server. + assert.match(body, /allowCustomInput/, "allowCustomInput field should be set on the event"); + assert.match(body, /allowCustomInput,$/m, "allowCustomInput should be passed through to the event object"); +}); diff --git a/tests/providers/session-revert-handler.test.mjs b/tests/providers/session-revert-handler.test.mjs new file mode 100644 index 0000000..69c9ef2 --- /dev/null +++ b/tests/providers/session-revert-handler.test.mjs @@ -0,0 +1,178 @@ +/** + * Regression: handleUndoMessageChanges must extract revertState from the + * server response and forward it to the webview so the revert banner can + * be shown with the correct message/part identifiers. + * + * Bug: Previously, after a successful `client.session.revert` call, the + * provider reloaded the session but never communicated the revert metadata + * (messageID, partID, snapshot, diff) back to the webview. The user clicked + * "Undo", the session reloaded, but no "Changes reverted — Restore" banner + * appeared because the webview had no revertState to render. + * + * Fix: handleUndoMessageChanges now reads `result.data.revert`, builds a + * revertState object {messageID, partID?, snapshot?, diff?}, and posts + * {type:"revertStateUpdate", revertState} to the webview before reloading. + * + * The ThrowOnError=false pattern is also critical: HTTP errors land in + * `result.error`, NOT in the catch block, so the handler must check + * `result.error` explicitly before accessing `result.data`. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readSource, joinFromRoot, extractFunctionBody } from "../helpers/source-utils.mjs"; + +const providerSource = readSource( + [joinFromRoot("src", "providers", "ChatViewProvider.ts")], + "ChatViewProvider.ts", +); + +function extractHandleUndoMessageChanges(source) { + return extractFunctionBody( + source, + "private async handleUndoMessageChanges(", + ); +} + +test("handleUndoMessageChanges calls client.session.revert with sessionID and messageID", () => { + const body = extractHandleUndoMessageChanges(providerSource); + assert.ok(body.length > 0, "handleUndoMessageChanges must exist"); + + assert.match( + body, + /client\.session\.revert\(\{[\s\S]*?sessionID:\s*targetSessionId,[\s\S]*?messageID:\s*targetMessageId/s, + "must call client.session.revert with both sessionID and messageID", + ); +}); + +test("handleUndoMessageChanges checks result.error before accessing result.data (ThrowOnError=false)", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + // The SDK defaults to ThrowOnError=false, so HTTP errors land in result.error + // not in the catch block. The handler MUST check result.error first. + assert.match( + body, + /revertResult as unknown as \{ error\?: unknown \}/, + "must cast revertResult to check for error field", + ); + + assert.match( + body, + /if \(revertError\)/, + "must branch on revertError presence before reading data", + ); + + assert.match( + body, + /showErrorMessage\([\s\S]*?Failed to undo changes:/s, + "must show error message to user when server returns error", + ); +}); + +test("handleUndoMessageChanges extracts revertState from result.data.revert", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + // The server returns { data: { revert: { messageID, partID, snapshot, diff } } } + assert.match( + body, + /\)\?\.data/, + "must access revertResult.data for the response payload", + ); + + assert.match( + body, + /\?\.revert\s*\?\?\s*undefined/, + "must read the revert field from session data", + ); + + assert.match( + body, + /revertField && typeof revertField === "object"/, + "must validate revert field is an object before using it", + ); +}); + +test("handleUndoMessageChanges builds revertState with messageID fallback to targetMessageId", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + assert.match( + body, + /const revertState = revertRecord/, + "must build revertState only when revertRecord exists", + ); + + assert.match( + body, + /messageID:\s*optionalStr\("messageID"\)\s*\?\?\s*targetMessageId/, + "revertState.messageID must fall back to targetMessageId when server omits it", + ); + + assert.match( + body, + /partID:\s*optionalStr\("partID"\)/, + "revertState must include optional partID", + ); + + assert.match( + body, + /snapshot:\s*optionalStr\("snapshot"\)/, + "revertState must include optional snapshot", + ); + + assert.match( + body, + /diff:\s*optionalStr\("diff"\)/, + "revertState must include optional diff", + ); +}); + +test("handleUndoMessageChanges posts revertStateUpdate to webview before reloading", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + assert.match( + body, + /postMessage\(\{[\s\S]*?type:\s*"revertStateUpdate"[\s\S]*?revertState[\s\S]*?\}\)/s, + "must post revertStateUpdate message with revertState to webview", + ); + + // The postMessage MUST come BEFORE handleLoadSession so the banner is shown + // immediately, not after the session reload finishes. + const postMessageIndex = body.indexOf('type: "revertStateUpdate"'); + const loadSessionIndex = body.indexOf("handleLoadSession("); + assert.ok( + postMessageIndex > -1 && loadSessionIndex > postMessageIndex, + "revertStateUpdate postMessage must occur before handleLoadSession reload", + ); +} + +test("handleUndoMessageChanges reloads session and sessions list after revert", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + assert.match( + body, + /await this\.handleLoadSession\(targetSessionId\)/, + "must reload the session after revert", + ); + + assert.match( + body, + /await this\.handleGetSessions\(\)/, + "must refresh sessions list after revert", + ); +}); + +test("handleUndoMessageChanges shows warning when missing message or session identifier", () => { + const body = extractHandleUndoMessageChanges(providerSource); + + assert.match( + body, + /if \(!targetMessageId \|\| !targetSessionId\)/, + "must guard against missing message or session identifiers", + ); + + assert.match( + body, + /showWarningMessage\([\s\S]*?Unable to undo changes: missing message or session identifier/s, + "must show warning message when identifiers are missing", + ); +}); diff --git a/tests/providers/session-unrevert-handler.test.mjs b/tests/providers/session-unrevert-handler.test.mjs new file mode 100644 index 0000000..a58efc7 --- /dev/null +++ b/tests/providers/session-unrevert-handler.test.mjs @@ -0,0 +1,264 @@ +import { describe, test } from 'node:test'; +import assert from 'node:assert/strict'; + +import { readSource, extractFunctionBody, joinFromRoot } from '../helpers/source-utils.mjs'; + +/** + * Session Unrevert (Restore) Handler — Integration Tests + * + * Guards the "Restore reverted messages" feature that calls the OpenCode SDK's + * session.unrevert endpoint. Parity with Codex's undo/restore behavior. + * + * Regression triggers: + * - handleUnrevertSession method removed or renamed + * - Message handler case "unrevertSession" removed + * - SDK call shape changed (sessionID path param, directory query param) + * - Post-unrevert session reload skipped (stale UI) + */ + +const source = readSource( + [joinFromRoot('src', 'providers', 'ChatViewProvider.ts')], + 'ChatViewProvider.ts', +); + +// ============================================================================ +// Layer 1: Message Handler Wiring +// ============================================================================ + +describe('unrevertSession message handler wiring', () => { + test('case "unrevertSession" exists in message switch', () => { + assert.match( + source, + /case\s+"unrevertSession"/, + 'ChatViewProvider must handle the "unrevertSession" webview message type', + ); + }); + + test('unrevertSession case calls handleUnrevertSession', () => { + const caseBody = extractFunctionBody(source, ' case "unrevertSession": {'); + assert.ok(caseBody, 'unrevertSession case body must exist'); + assert.match( + caseBody, + /handleUnrevertSession\(/, + 'case must call handleUnrevertSession method', + ); + }); +}); + +// ============================================================================ +// Layer 2: handleUnrevertSession Method +// ============================================================================ + +describe('handleUnrevertSession method', () => { + test('method exists as private async', () => { + assert.match( + source, + /private\s+async\s+handleUnrevertSession\(/, + 'handleUnrevertSession must be a private async method', + ); + }); + + test('accepts optional requestedSessionId parameter', () => { + assert.match( + source, + /handleUnrevertSession\(\s*requestedSessionId\?:\s*string/, + 'handleUnrevertSession must accept optional requestedSessionId for routing', + ); + }); + + test('resolves target session ID with firstNonEmptyString fallback', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.ok(body, 'method body must exist'); + assert.match( + body, + /firstNonEmptyString\(\s*requestedSessionId,\s*this\.currentSessionId/, + 'must fall back to currentSessionId when requestedSessionId is empty', + ); + }); + + test('returns early with a visible warning when no session ID is available', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /if\s*\(!targetSessionId\)\s*\{[\s\S]*?showWarningMessage/, + 'must show a warning to the user when targetSessionId is falsy', + ); + }); +}); + +// ============================================================================ +// Layer 4b: RequestResult Error Checking (ThrowOnError=false) +// ============================================================================ + +describe('handleUnrevertSession checks result.error before data', () => { + test('checks unrevertResult.error before proceeding', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /unrevertError/, + 'must extract error from the SDK result', + ); + assert.match( + body, + /if\s*\(\s*unrevertError\s*\)/, + 'must check the extracted error before accessing data', + ); + }); + + test('shows error message when SDK returns an HTTP error', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /showErrorMessage\([\s\S]*?Failed to restore/, + 'must show a user-facing error when the SDK returns an HTTP error', + ); + }); +}); + +// ============================================================================ +// Layer 3: SDK Call Shape +// ============================================================================ + +describe('handleUnrevertSession SDK call', () => { + test('awaits ensureRunning before making the SDK call', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /const\s+client\s*=\s*await\s+this\.serverManager\.ensureRunning\(\)/, + 'must wait for server readiness before unrevert', + ); + }); + + test('calls client.session.unrevert with sessionID path param', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /client\.session\.unrevert\(\s*\{[\s\S]*?sessionID:\s*targetSessionId/, + 'must call client.session.unrevert with sessionID path parameter', + ); + }); + + test('passes workspace directory as query param when available', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /getWorkspaceDirectory\(\)/, + 'must resolve workspace directory', + ); + assert.match( + body, + /directory:\s*workspaceDir/, + 'must pass directory as query parameter to unrevert', + ); + }); + + test('does NOT send a body (unrevert is bodyless per SDK contract)', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + // The unrevert call should only have sessionID (path) and directory (query) — no body + const unrevertMatch = body.match(/client\.session\.unrevert\(\s*\{([\s\S]*?)\}\s*\)/); + assert.ok(unrevertMatch, 'unrevert call must exist'); + assert.doesNotMatch( + unrevertMatch[1], + /body:/, + 'unrevert must NOT include a body parameter (SDK contract: body is never)', + ); + }); +}); + +// ============================================================================ +// Layer 4: Post-Unrevert Reload + Error Handling +// ============================================================================ + +describe('handleUnrevertSession post-call behavior', () => { + test('reloads the session after unrevert', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /handleLoadSession\(targetSessionId\)/, + 'must reload the session to refresh message list after unrevert', + ); + }); + + test('refreshes the session list after unrevert', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /handleGetSessions\(\)/, + 'must refresh the sessions list after unrevert', + ); + }); + + test('catches errors and shows user-facing error message', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /catch\s*\(\s*error\s*\)/, + 'must have a catch block for error handling', + ); + assert.match( + body, + /showErrorMessage\(/, + 'must show an error message to the user on failure', + ); + assert.match( + body, + /Failed to restore/, + 'error message must be user-facing ("Failed to restore...")', + ); + }); + + test('logs the error with session context', () => { + const body = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.match( + body, + /this\.logger\.error\(/, + 'must log the error for diagnostics', + ); + assert.match( + body, + /sessionId:\s*targetSessionId/, + 'error log must include the sessionId for traceability', + ); + }); +}); + +// ============================================================================ +// Layer 5: Parity with handleUndoMessageChanges +// ============================================================================ + +describe('unrevert handler mirrors revert handler structure', () => { + test('both handlers use the same ensureRunning + directory resolution pattern', () => { + const undoBody = extractFunctionBody(source, ' private async handleUndoMessageChanges('); + const unrevertBody = extractFunctionBody(source, ' private async handleUnrevertSession('); + assert.ok(undoBody, 'handleUndoMessageChanges must exist'); + assert.ok(unrevertBody, 'handleUnrevertSession must exist'); + + // Both must resolve workspace directory the same way + assert.match(undoBody, /getWorkspaceDirectory\(\)/); + assert.match(unrevertBody, /getWorkspaceDirectory\(\)/); + + // Both must reload session + refresh sessions list + assert.match(undoBody, /handleLoadSession\(/); + assert.match(unrevertBody, /handleLoadSession\(/); + assert.match(undoBody, /handleGetSessions\(\)/); + assert.match(unrevertBody, /handleGetSessions\(\)/); + }); + + test('both handlers check result.error before accessing data (ThrowOnError=false)', () => { + const undoBody = extractFunctionBody(source, ' private async handleUndoMessageChanges('); + const unrevertBody = extractFunctionBody(source, ' private async handleUnrevertSession('); + + assert.match(undoBody, /revertError/, 'revert handler must extract error from result'); + assert.match(undoBody, /if\s*\(\s*revertError\s*\)/, 'revert handler must check extracted error'); + assert.match(unrevertBody, /unrevertError/, 'unrevert handler must extract error from result'); + assert.match(unrevertBody, /if\s*\(\s*unrevertError\s*\)/, 'unrevert handler must check extracted error'); + }); + + test('both handlers show showWarningMessage on early return', () => { + const undoBody = extractFunctionBody(source, ' private async handleUndoMessageChanges('); + const unrevertBody = extractFunctionBody(source, ' private async handleUnrevertSession('); + + assert.match(undoBody, /showWarningMessage/, 'revert handler must show warning on early return'); + assert.match(unrevertBody, /showWarningMessage/, 'unrevert handler must show warning on early return'); + }); +}); diff --git a/tests/unit/centralized-filter-normalizer-paths.test.mjs b/tests/unit/centralized-filter-normalizer-paths.test.mjs new file mode 100644 index 0000000..3302362 --- /dev/null +++ b/tests/unit/centralized-filter-normalizer-paths.test.mjs @@ -0,0 +1,184 @@ +/** + * Regression: Centralized debug payload filter normalizer must check + * syncEvent.data.type paths for correct live-only event classification. + * + * Bug: The normalizedCentralizedEventType() function checked type/event/kind, + * payload.type, syncEvent.type, and payload.syncEvent.type — but NOT + * syncEvent.data.type. Many real SSE events carry the event type at + * syncEvent.data.type, so they resolved to empty type strings and were + * treated as generic persist events instead of live-only. + * + * Fix: Added syncEvent.data.type, syncEvent.data.event, syncEvent.data.kind, + * payload.syncEvent.data.type, payload.syncEvent.data.event, and + * payload.syncEvent.data.kind to the normalizer fallback chain. + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readSource, extractFunctionBody, joinFromRoot } from "../helpers/source-utils.mjs"; + +const filterSource = readSource( + [joinFromRoot("src", "shared", "centralizedDebugPayloadFilter.ts")], + "centralizedDebugPayloadFilter.ts", +); + +const normalizerBody = extractFunctionBody( + filterSource, + "export function normalizedCentralizedEventType(", +); + +test("normalizer is exported (not internal-only)", () => { + assert.match( + filterSource, + /export function normalizedCentralizedEventType\(/, + "must be exported for reuse by live-event parsers", + ); +}); + +test("normalizer checks direct type/event/kind fields", () => { + assert.match( + normalizerBody, + /event\.type \?\? event\.event \?\? event\.kind/, + "must check top-level type/event/kind", + ); +}); + +test("normalizer checks syncEvent.data.type path", () => { + assert.match( + normalizerBody, + /syncData\?\.type/, + "must check syncEvent.data.type — this was the missing path", + ); + + assert.match( + normalizerBody, + /syncData\?\.event/, + "must check syncEvent.data.event", + ); + + assert.match( + normalizerBody, + /syncData\?\.kind/, + "must check syncEvent.data.kind", + ); +}); + +test("normalizer checks payload.syncEvent.data.type path", () => { + assert.match( + normalizerBody, + /payloadSyncData\?\.type/, + "must check payload.syncEvent.data.type — this was the missing path", + ); + + assert.match( + normalizerBody, + /payloadSyncDataType/, + "must use payloadSyncDataType in the fallback chain", + ); +}); + +test("normalizer strips numeric suffixes from event types", () => { + assert.ok( + normalizerBody.includes('.replace(/'), + "must strip numeric suffixes from event types via replace", + ); + assert.ok( + normalizerBody.includes('d+$'), + "must match digit pattern at end of event type string", + ); +}); + +test("exclusion rules include syncEvent.data.type and payload.syncEvent.data.type paths", () => { + assert.match( + filterSource, + /syncEvent\.data\.type/, + "exclusion rules must include syncEvent.data.type path", + ); + + assert.match( + filterSource, + /payload\.syncEvent\.data\.type/, + "exclusion rules must include payload.syncEvent.data.type path", + ); +}); + +test("session.status, tui.show, and tui.toast.show are in exclusion rules", () => { + const exclusionSection = filterSource.slice( + filterSource.indexOf("CENTRALIZED_DEBUG_EXCLUDED_PATH_RULES"), + filterSource.indexOf("function"), + ); + + for (const eventType of ["session.status", "tui.show", "tui.toast.show"]) { + assert.ok( + exclusionSection.includes(`"${eventType}"`), + `exclusion rules must include "${eventType}"`, + ); + } +}); + +test("disposition returns live-only for session.status and tui events", () => { + const dispositionBody = extractFunctionBody( + filterSource, + "export function getCentralizedDebugPayloadDisposition(", + ); + + assert.match( + dispositionBody, + /session\.status/, + "disposition must recognize session.status", + ); + + assert.match( + dispositionBody, + /tui\.toast\.show/, + "disposition must recognize tui.toast.show", + ); + + assert.match( + dispositionBody, + /tui\.show/, + "disposition must recognize tui.show", + ); + + assert.match( + dispositionBody, + /"live-only"/, + "must return live-only for these event types", + ); +}); + +test("messageHandler only appends to centralized tape when disposition is persist", () => { + const handlerSource = readSource( + [joinFromRoot("webview", "shared", "src", "chat", "lib", "messageHandler.ts")], + "messageHandler.ts", + ); + + // streamEvent path + const streamEventSection = handlerSource.slice( + handlerSource.indexOf('case "streamEvent"'), + handlerSource.indexOf('case "streamEventBatch"'), + ); + + assert.match( + streamEventSection, + /centralizedDisposition === "persist"/, + "streamEvent must only append to centralized tape when disposition is persist", + ); + + assert.doesNotMatch( + streamEventSection, + /centralizedDisposition !== "excluded-noise"/, + "streamEvent must NOT use the old excluded-noise check (that was the leak)", + ); + + // streamEventBatch path + const batchSection = handlerSource.slice( + handlerSource.indexOf('case "streamEventBatch"'), + ); + + assert.match( + batchSection, + /disposition === "persist"/, + "streamEventBatch must only append to centralized tape when disposition is persist", + ); +}); diff --git a/tests/webview/activity-timeline-attached-payloads-regression.test.mjs b/tests/webview/activity-timeline-attached-payloads-regression.test.mjs index 6a5efd3..93a65e3 100644 --- a/tests/webview/activity-timeline-attached-payloads-regression.test.mjs +++ b/tests/webview/activity-timeline-attached-payloads-regression.test.mjs @@ -16,7 +16,7 @@ test("message-attached centralized payloads keep scoped events that lack explici assert.match( body, - /const attachedPayloadsWithoutIds = messageAttachedRawSdkEventPayloads\.filter\(\(event\) =>\s*!extractEventMessageId\(event\),\s*\);/s, + /const attachedPayloadsWithoutIds = messageAttachedRawSdkEventPayloads\.filter\(\(event\) =>\s*!extractSemanticEventMessageId\(event\),\s*\);/s, "message-attached payload selection should explicitly preserve already-scoped entries that do not expose messageID/messageId", ); @@ -40,6 +40,18 @@ test("assistant cards keep assistant-like no-id centralized terminal payloads fr "MessageComponents should define a dedicated guard for assistant-like centralized payloads that have no explicit message id", ); + assert.match( + source, + /function extractSemanticEventMessageId\(event: unknown\): string \| null \{[\s\S]*fallbackId\.toLowerCase\(\)\.startsWith\("evt_"\)/, + "assistant-card scoping should distinguish semantic message ids from wrapper evt_ ids before treating a payload as message-scoped", + ); + + assert.match( + source, + /wrapper event id such as `evt_\*`[\s\S]*NOT safe for[\s\S]*assistant-card scoping/s, + "MessageComponents should document why wrapper evt_ ids must not be treated as semantic assistant message ids", + ); + const body = extractFunctionBody( source, "const centralizedRawSdkEventPayloads = useMemo(() => {", @@ -57,3 +69,29 @@ test("assistant cards keep assistant-like no-id centralized terminal payloads fr "message-specific centralized events should merge assistant-like no-id session payloads before attached no-id payloads", ); }); + +test("assistant cards keep no-id session metadata needed for header labels", () => { + assert.match( + source, + /const isAssistantHeaderMetadataEvent =\s*eventType === "session\.next\.agent\.switched" \|\|\s*eventType === "session\.next\.model\.switched" \|\|\s*eventType === "session\.updated";/s, + "assistant no-id payload guard should treat session-level agent/model updates as header metadata events", + ); + + assert.match( + source, + /const hasAssistantHeaderMetadata =\s*!!firstNonEmptyString\([\s\S]*info\?\.agent,[\s\S]*info\?\.providerID,[\s\S]*info\?\.modelID,[\s\S]*info\?\.variant,[\s\S]*\);/s, + "assistant no-id payload guard should detect header metadata fields on session-scoped events", + ); + + assert.match( + source, + /\|\| \(isAssistantHeaderMetadataEvent && hasAssistantHeaderMetadata\);/s, + "assistant no-id payload guard should preserve header metadata events even when they do not carry text or structured output", + ); + + assert.match( + source, + /level metadata rows like `session\.updated` and `session\.next\.\*` stop looking[\s\S]*top response header loses agent\/model\/[\s\S]*thinking labels after hydration/s, + "MessageComponents should document the hydration regression that occurs when session metadata is mis-scoped", + ); +}); diff --git a/tests/webview/assistant-header-thinking-variant.test.mjs b/tests/webview/assistant-header-thinking-variant.test.mjs index 80a25c2..ff69000 100644 --- a/tests/webview/assistant-header-thinking-variant.test.mjs +++ b/tests/webview/assistant-header-thinking-variant.test.mjs @@ -14,12 +14,31 @@ const messageHandlerSource = readSource( ); test("assistant header renders thinking variant beside agent/model metadata", () => { - // Implementation detail test simplified - component structure is implementation detail assert.match( messageComponentsSource, /thinking|variant|level/, "should handle thinking variant metadata", ); + assert.match( + messageComponentsSource, + /const assistantHeaderAgentLabel = firstNonEmptyString\(agentName\)\?\.trim\(\) \|\| "assistant";/, + "assistant response header should keep a visible fallback agent label instead of disappearing when metadata is partially missing", + ); + assert.match( + messageComponentsSource, + /const showAssistantResponseHeader = hasPrimaryResponseBody;/, + "assistant response header should render for every visible AI response block that has a primary response body", + ); + assert.match( + messageComponentsSource, + /if \(assistantHeaderAgentLabel\) \{[\s\S]*key: "agent",[\s\S]*text: assistantHeaderAgentLabel,/s, + "assistant response header should always emit a visible leading agent segment using the fallback-aware label", + ); + assert.match( + messageComponentsSource, + /modelName !== "assistant" &&[\s\S]*modelName !== assistantHeaderAgentLabel/s, + "assistant response header should avoid duplicating the fallback assistant label as both agent and model", + ); }); test("streaming and normalized messages persist thinking variant metadata", () => { diff --git a/tests/webview/assistant-message-collapsed-logic.test.mjs b/tests/webview/assistant-message-collapsed-logic.test.mjs index 69a67fa..b001778 100644 --- a/tests/webview/assistant-message-collapsed-logic.test.mjs +++ b/tests/webview/assistant-message-collapsed-logic.test.mjs @@ -16,8 +16,9 @@ function getCollapsedRenderingState({ isStreamingActive, hasCopyableResponseContent, }) { - // 1. Header is only shown for the last message in a block - const showAssistantResponseHeader = hasPrimaryResponseBody && isLastInBlock; + // 1. Every visible AI response block should keep its header so agent/model + // metadata remains visible even on expanded intermediate cards. + const showAssistantResponseHeader = hasPrimaryResponseBody; // 2. The timestamp is moved inside the bubble and copy button removed for messages WITHOUT copyable text const showTimestampInsideBubble = !isStreamingActive && !hasCopyableResponseContent; @@ -46,7 +47,7 @@ describe("Assistant Message Collapsed State UI Logic", () => { assert.strictEqual(state.showFooterOutsideBubble, false, "Copy button and external footer should be hidden for intermediate steps"); }); - it("should hide header but put footer outside for intermediate steps WITH text (e.g. expanded text message followed by tool call)", () => { + it("should keep header visible for intermediate steps WITH text (e.g. expanded text message followed by tool call)", () => { const state = getCollapsedRenderingState({ hasPrimaryResponseBody: true, isLastInBlock: false, // It's an expanded text message that isn't the final card @@ -54,7 +55,7 @@ describe("Assistant Message Collapsed State UI Logic", () => { hasCopyableResponseContent: true, // It has text }); - assert.strictEqual(state.showAssistantResponseHeader, false, "Header should be hidden because it's not the last card"); + assert.strictEqual(state.showAssistantResponseHeader, true, "Header should remain visible for any response block that has body text"); assert.strictEqual(state.showTimestampInsideBubble, false, "Timestamp should NOT be inside the bubble because it has text"); assert.strictEqual(state.showFooterOutsideBubble, true, "Copy button and external footer should be visible because it has text"); }); @@ -71,6 +72,17 @@ describe("Assistant Message Collapsed State UI Logic", () => { assert.strictEqual(state.showTimestampInsideBubble, false, "Timestamp should NOT be inside the bubble for the final message"); assert.strictEqual(state.showFooterOutsideBubble, true, "Copy button and external footer should be visible for the final message"); }); + + it("should hide header only when there is no primary response body", () => { + const state = getCollapsedRenderingState({ + hasPrimaryResponseBody: false, + isLastInBlock: true, + isStreamingActive: false, + hasCopyableResponseContent: false, + }); + + assert.strictEqual(state.showAssistantResponseHeader, false, "Header should stay hidden for non-response activity-only cards"); + }); it("should hide footers while streaming is active", () => { const state = getCollapsedRenderingState({ diff --git a/tests/webview/centralized-render-source-of-truth-regression.test.mjs b/tests/webview/centralized-render-source-of-truth-regression.test.mjs index fbdf8b1..dd41869 100644 --- a/tests/webview/centralized-render-source-of-truth-regression.test.mjs +++ b/tests/webview/centralized-render-source-of-truth-regression.test.mjs @@ -34,7 +34,7 @@ test("chat shell display path passes only centralized data into transcript build ); assert.match( chatShellSource, - /function buildCentralizedRenderMessages\(\s*rawSdkEventPayloads: unknown\[\]\s*\): Message\[\]/s, + /function buildCentralizedRenderMessages\(\s*rawSdkEventPayloads: unknown\[\],[\s\S]*?\): Message\[\]/s, "render message builder should not accept local messages as an input", ); assert.match( @@ -44,7 +44,7 @@ test("chat shell display path passes only centralized data into transcript build ); assert.match( chatShellSource, - /const transcriptProjection = useMemo\(\s*\(\) => buildCentralizedTranscriptProjection\(centralizedSessionRawSdkEventPayloads\),/s, + /const transcriptProjection = useMemo\(\s*\(\) => buildCentralizedTranscriptProjection\((?:centralizedSessionRawSdkEventPayloads|throttledPayloads)\),/s, "ChatContent should derive transcript state from one centralized projection pass", ); assert.match( @@ -73,3 +73,131 @@ test("chat shell display path passes only centralized data into transcript build "rendered transcript components should receive centralized renderMessages, not local state.messages", ); }); + +test("centralized render builder preserves user file parts instead of dropping non-text attachments", () => { + const body = extractFunctionBody( + chatShellSource, + "function buildCentralizedRenderMessages(", + ); + + assert.match( + body, + /if \(messageId\) \{[\s\S]*?const existingParts = partsByMessageId\.get\(messageId\) \?\? \[\];[\s\S]*?existingParts\.push\(part\);[\s\S]*?partsByMessageId\.set\(messageId, existingParts\);[\s\S]*?\}/s, + "every centralized part event with a messageId should be preserved in partsByMessageId before role-specific filtering", + ); + assert.match( + body, + /parts:\s*getPartsForMessageId\(descriptor\.messageId\),/s, + "merged user messages must receive preserved centralized parts so attachment chips can render", + ); + assert.match( + body, + /if \(messageId\) \{[\s\S]*?partsByMessageId\.set\(messageId, existingParts\);[\s\S]*?\}[\s\S]*?if \(firstNonEmptyString\(part\?\.type\)\?\.toLowerCase\(\) !== "text"\) \{\s*continue;\s*\}/s, + "user attachment parts must be preserved before the text-only descriptor branch runs", + ); +}); + +test("centralized user messages derive visible text from parts and ignore synthetic tool echoes", () => { + assert.match( + chatShellSource, + /function buildVisibleUserMessageText\(/, + "centralized user-message builder should derive visible text through a dedicated helper", + ); + assert.match( + chatShellSource, + /\.filter\(\(part\) => part\?\.synthetic !== true\)/, + "centralized user-text derivation should exclude synthetic user parts before building visible text", + ); + assert.match( + chatShellSource, + /normalized\.startsWith\("called the "\)[\s\S]*?normalized\.includes\(" tool with the following input:"\)/, + "centralized user-text derivation should ignore synthetic tool-call echo text", + ); + assert.match( + chatShellSource, + /value\.includes\(""\)[\s\S]*?value\.includes\("<\/path>"\)[\s\S]*?value\.includes\(""\)/, + "centralized user-text derivation should ignore raw file-dump payload text", + ); + assert.match( + chatShellSource, + /const visibleUserText = buildVisibleUserMessageText\(descriptor\.text, userParts\);[\s\S]*?content:\s*visibleUserText,[\s\S]*?text:\s*visibleUserText,[\s\S]*?parts:\s*userParts/s, + "canonical user messages should use filtered visible text while preserving attachment parts for the same bubble", + ); +}); + +test("centralized transcript projection emits session-error entries in tape order", () => { + assert.match( + chatShellSource, + /parseCentralizedSessionErrorEvent\([\s\S]*?kind:\s*"session\.error"/s, + "transcript projection must parse centralized session errors into ordered conversation entries", + ); + assert.match( + chatShellSource, + /hasPrimarySessionErrorEvent[\s\S]*?candidateError\.source !== "message\.updated"/s, + "projection should detect when a primary session.error/error event exists", + ); + assert.match( + chatShellSource, + /hasSpecificSessionErrorEvent[\s\S]*?isGenericSessionErrorMessage\(candidateError\.message\)/s, + "projection should prefer specific error text over generic fallback messages", + ); + assert.match( + chatShellSource, + /conversationEntries:\s*conversationEntries\.sort\(\(left, right\) => left\.order - right\.order\)/, + "session-error entries must participate in the same centralized sort as the rest of the transcript", + ); +}); + +test("centralized transcript projection no longer emits session-status entries", () => { + const projectionBody = extractFunctionBody( + chatShellSource, + "function buildCentralizedTranscriptProjection(", + ); + + assert.doesNotMatch( + chatShellSource, + /function parseCentralizedSessionStatusEvent\(/, + "centralized transcript layer should no longer define a dedicated session.status parser", + ); + assert.doesNotMatch( + projectionBody, + /kind:\s*"session\.status"/, + "transcript projection must not emit session.status conversation entries anymore", + ); + assert.doesNotMatch( + chatShellSource, + /entry\.kind === "session\.status"[\s\S]*?entry\.status\.createdAt \?\? 0/s, + "pending-user merge should no longer treat session.status as a centralized conversation entry", + ); +}); + +test("centralized builder classifies system messages as full-width entries instead of defaulting to user bubbles", () => { + const body = extractFunctionBody( + chatShellSource, + "function buildCentralizedRenderMessages(", + ); + + assert.match( + body, + /if \(messageId && !messageRolesById\.has\(messageId\)\) \{[\s\S]*?partEventRole[\s\S]*?systemMessageIds\.add\(messageId\)/s, + "role must be registered from message.part.updated events so system messages without a matching message.updated are not lost", + ); + + assert.match( + body, + /looksLikeStandaloneSystemMessage\(descriptor\.text\)/, + "routing loop must fall back to content-based system detection for descriptors with no registered role", + ); + + assert.match( + chatShellSource, + /function looksLikeStandaloneSystemMessage\([\s\S]*?\/\^\\\[\[a-z\]/s, + "content-based fallback should detect bracketed system message patterns", + ); + + assert.match( + body, + /role:\s*"system",[\s\S]*?info:\s*\{[\s\S]*?role:\s*"system"/s, + "system descriptors must be merged with role and info.role set to system for full-width rendering", + ); +}); diff --git a/tests/webview/code-selection-preview-modal.test.mjs b/tests/webview/code-selection-preview-modal.test.mjs new file mode 100644 index 0000000..12404c4 --- /dev/null +++ b/tests/webview/code-selection-preview-modal.test.mjs @@ -0,0 +1,164 @@ +/** + * Regression: CodeSelectionPreviewModal must render highlighted code in a + * portal with accessible title/line labels and keyboard escape dismissal. + * + * Context: When a user clicks a code-selection chip on their message, a + * preview modal opens showing the selected code with syntax highlighting. + * The modal must: + * 1. Build a readable title (filename:lineInfo) from sparse chip data + * 2. Build a line label for single vs range selections + * 3. Close on Escape key and backdrop click + * 4. Use hljs for syntax highlighting with language fallback + * 5. Render via createPortal to document.body + */ + +import test from "node:test"; +import assert from "node:assert/strict"; +import { readSource, joinFromRoot, extractFunctionBody } from "../helpers/source-utils.mjs"; + +const modalSource = readSource( + [joinFromRoot("webview", "shared", "src", "chat", "CodeSelectionPreviewModal.tsx")], + "CodeSelectionPreviewModal.tsx", +); + +test("CodeSelectionPreviewData interface defines all chip data fields", () => { + assert.match( + modalSource, + /interface CodeSelectionPreviewData/, + "must export CodeSelectionPreviewData interface", + ); + assert.match( + modalSource, + /path\?:\s*string/, + "must have optional path field", + ); + assert.match( + modalSource, + /filename\?:\s*string/, + "must have optional filename field", + ); + assert.match( + modalSource, + /languageId\?:\s*string/, + "must have optional languageId field", + ); + assert.match( + modalSource, + /lineInfo\?:\s*string/, + "must have optional lineInfo field", + ); + assert.match( + modalSource, + /content:\s*string/, + "must have required content field", + ); + assert.match( + modalSource, + /startLine\?:\s*number/, + "must have optional startLine field", + ); + assert.match( + modalSource, + /endLine\?:\s*number/, + "must have optional endLine field", + ); +}); + +test("buildLineLabel formats single and range line numbers correctly", () => { + const body = extractFunctionBody(modalSource, "function buildLineLabel("); + assert.ok(body.length > 0, "buildLineLabel must exist"); + + // No start/end → fall back to lineInfo or empty + assert.match( + body, + /lineInfo \?\? ""/, + "must fall back to lineInfo string when no numeric lines", + ); + + // start && end && start !== end → "start-end" + assert.match( + body, + /\$\{startLine\}-\$\{endLine\}/, + "must format range as start-end when start differs from end", + ); +}); + +test("buildTitle combines filename and line info into modal title", () => { + const body = extractFunctionBody(modalSource, "function buildTitle("); + assert.ok(body.length > 0, "buildTitle must exist"); + + // name falls back: filename ?? path ?? "code-selection" + assert.match( + body, + /filename \?\? data\.path \?\? "code-selection"/, + "must derive name from filename, path, or fallback to code-selection", + ); + + // If lines exist: `${name}:${lines}`, else just name + assert.match( + body, + /lines \? `\$\{name\}:\$\{lines\}` : name/, + "must append line info to title with colon separator", + ); +}); + +test("modal closes on Escape key", () => { + assert.match( + modalSource, + /event\.key === "Escape"/, + "must listen for Escape key to close modal", + ); + assert.match( + modalSource, + /onClose\(\)/, + "must call onClose when Escape is pressed", + ); +}); + +test("modal uses hljs with language fallback to auto-detection", () => { + assert.match( + modalSource, + /hljs\.getLanguage\(/, + "must check if hljs recognizes the language", + ); + assert.match( + modalSource, + /hljs\.highlight\(/, + "must use hljs.highlight when language is known", + ); + assert.match( + modalSource, + /hljs\.highlightAuto\(/, + "must fall back to hljs.highlightAuto for unknown languages", + ); +}); + +test("modal renders via createPortal to document.body", () => { + assert.match( + modalSource, + /createPortal\(/, + "must use createPortal for proper z-index stacking", + ); + assert.match( + modalSource, + /document\.body/, + "must portal into document.body", + ); +}); + +test("modal respects isOpen prop and does not render when closed", () => { + assert.match( + modalSource, + /if \(!isOpen\) return null/, + "must bail early when isOpen is false", + ); +}); + +test("modal closes on backdrop click", () => { + // Backdrop click should close, but content click should not + assert.match( + modalSource, + /onClick=\{onClose\}/, + "backdrop must call onClose on click", + ); +}); diff --git a/tests/webview/code-selection-rendering.test.mjs b/tests/webview/code-selection-rendering.test.mjs new file mode 100644 index 0000000..ffe6e0b --- /dev/null +++ b/tests/webview/code-selection-rendering.test.mjs @@ -0,0 +1,178 @@ +/** + * Regression: Code selections attached to user messages must render as + * clickable chips that open a preview modal, not as inline text or + * dropped attachments. + * + * Bug: The new code-selection part type (type:"file" with source.type:"file", + * source.lineInfo, source.text.value) was being misclassified by + * isExplicitFileAttachmentPart as a regular file attachment, so it showed + * as a plain file chip with no preview capability. The selection content + * was invisible to the user. + * + * Fix: MessageComponents now has: + * 1. isCodeSelectionPart() — distinguishes selection parts from file parts + * 2. collectCodeSelectionsFromParts() — builds CodeSelectionChipData[] + * 3. parseLineRange() + rangeToLines() — normalize line info + * 4. UserMessage renders clickable