diff --git a/bun.lock b/bun.lock index cca331ed..f733b04e 100644 --- a/bun.lock +++ b/bun.lock @@ -5,9 +5,9 @@ "": { "name": "hapi", "devDependencies": { - "@playwright/test": "^1.60.0", + "@playwright/test": "^1.61.0", "concurrently": "^9.2.1", - "playwright": "1.60.0", + "playwright": "1.61.0", "react-devtools-core": "^7.0.1", "vite-plugin-pwa": "^1.2.0", }, @@ -740,7 +740,7 @@ "@pinojs/redact": ["@pinojs/redact@0.4.0", "", {}, "sha512-k2ENnmBugE/rzQfEcdWHcCY+/FM3VLzH9cYEsbdsoqrvzAKRhUZeRNhAZvB8OitQJ1TBed3yqWtdjzS6wJKBwg=="], - "@playwright/test": ["@playwright/test@1.60.0", "", { "dependencies": { "playwright": "1.60.0" }, "bin": { "playwright": "cli.js" } }, "sha512-O71yZIbAh/PxDMNGns37GHBIfrVkEVyn+AXyIa5dOTfb4/xNvRWV+Vv/NMbNCtODB/pO7vLlF2OTmMVLhmr7Ag=="], + "@playwright/test": ["@playwright/test@1.61.0", "", { "dependencies": { "playwright": "1.61.0" }, "bin": { "playwright": "cli.js" } }, "sha512-cKA5B6lpFEMyMGjxF54QihfYpB4FkEGH+qZhtArDEG+wezQAJY8Pq6C7T1SjWz+FFzt3TbyoXBQYk/0292TdJA=="], "@primer/octicons": ["@primer/octicons@19.23.1", "", { "dependencies": { "object-assign": "^4.1.1" } }, "sha512-CzjGmxkmNhyst6EekrS3SJPdtzgIkUMP/LSJch65y99/kmiFXbO1a+q7zoYe3hnI9NaOM0IN+ydDIbOmd8YqcA=="], @@ -2482,9 +2482,9 @@ "pkg-types": ["pkg-types@1.3.1", "", { "dependencies": { "confbox": "^0.1.8", "mlly": "^1.7.4", "pathe": "^2.0.1" } }, "sha512-/Jm5M4RvtBFVkKWRu2BLUTNP8/M2a+UwuAX+ae4770q1qVGtfjG+WTCupoZixokjmHiry8uI+dlY8KXYV5HVVQ=="], - "playwright": ["playwright@1.60.0", "", { "dependencies": { "playwright-core": "1.60.0" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-hheHdokM8cdqCb0lcE3s+zT4t4W+vvjpGxsZlDnikarzx8tSzMebh3UiFtgqwFwnTnjYQcsyMF8ei2mCO/tpeA=="], + "playwright": ["playwright@1.61.0", "", { "dependencies": { "playwright-core": "1.61.0" }, "optionalDependencies": { "fsevents": "2.3.2" }, "bin": { "playwright": "cli.js" } }, "sha512-Z+7BeeqQPRRzklHsVFP4KTGIyMxKUmfeRA4WisM6G3/XW6nwGeX6fX9qYaDa+CiUqpOkb2f6X3nar05R3kSuJQ=="], - "playwright-core": ["playwright-core@1.60.0", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-9bW6zvX/m0lEbgTKJ6YppOKx8H3VOPBMOCFh2irXFOT4BbHgrx5hPjwJYLT40Lu+4qtD36qKc/Hn56StUW57IA=="], + "playwright-core": ["playwright-core@1.61.0", "", { "bin": { "playwright-core": "cli.js" } }, "sha512-caX7TrY3Ml6egyDX0WUcTHDxodl/b51y5wJOdCEA36QviK/s2g081hvmGs8eaE3DWb6NYZQ6BjO/QkNRPenoPA=="], "pngjs": ["pngjs@5.0.0", "", {}, "sha512-40QW5YalBNfQo5yRYmiw7Yz6TKKVr3h6970B2YE+3fQpsWcrbj1PzJgxeJ19DRQjhMbKPIuMY8rFaXc8moolVw=="], diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts index 74e3cf13..5d65e267 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.test.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.test.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from 'node:url'; import type { AgentMessage } from '@/agent/types'; import { AcpMessageHandler } from './AcpMessageHandler'; import { ACP_SESSION_UPDATE_TYPES } from './constants'; +import { clearGeneratedImages } from '@/modules/common/generatedImages'; function getToolResult(messages: AgentMessage[], id: string): Extract { const result = messages.find((message): message is Extract => @@ -2417,4 +2418,93 @@ describe('AcpMessageHandler', () => { }); }); }); + + it('emits generated_image agent messages from ACP image content blocks', async () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x00]); + + handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { + type: 'image', + mimeType: 'image/png', + data: pngHeader.toString('base64') + } + }); + + await vi.waitFor(() => { + expect(messages.some((message) => message.type === 'generated_image')).toBe(true); + }); + + const imageMessage = messages.find( + (message): message is Extract => + message.type === 'generated_image' + ); + expect(imageMessage?.mimeType).toBe('image/png'); + expect(imageMessage?.fileName).toBeTruthy(); + expect(imageMessage?.imageId).toBeTruthy(); + expect(imageMessage?.source).toEqual({ ingress: 'acp' }); + clearGeneratedImages(); + }); + + it('emits generated_image before later tool_call when image registration is async', async () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message)); + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x00]); + + await handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { + type: 'image', + mimeType: 'image/png', + data: pngHeader.toString('base64'), + }, + }); + await handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.toolCall, + toolCallId: 'call-after-image', + title: 'Read', + kind: 'read', + status: 'in_progress', + }); + + const imageIndex = messages.findIndex((message) => message.type === 'generated_image'); + const toolIndex = messages.findIndex((message) => message.type === 'tool_call'); + expect(imageIndex).toBeGreaterThanOrEqual(0); + expect(toolIndex).toBeGreaterThan(imageIndex); + clearGeneratedImages(); + }); + + it('emits buffered text before generated_image when text precedes an ACP image block', async () => { + const messages: AgentMessage[] = []; + const handler = new AcpMessageHandler((message) => messages.push(message), { flavor: 'cursor' }); + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x00]); + + await handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { type: 'text', text: 'Here is the screenshot:' } + }); + await handler.handleUpdate({ + sessionUpdate: ACP_SESSION_UPDATE_TYPES.agentMessageChunk, + content: { + type: 'image', + mimeType: 'image/png', + data: pngHeader.toString('base64') + } + }); + + await vi.waitFor(() => { + expect(messages.some((message) => message.type === 'generated_image')).toBe(true); + }); + + const textIndex = messages.findIndex((message) => message.type === 'text'); + const imageIndex = messages.findIndex((message) => message.type === 'generated_image'); + expect(textIndex).toBeGreaterThanOrEqual(0); + expect(imageIndex).toBeGreaterThan(textIndex); + if (messages[imageIndex]?.type === 'generated_image') { + expect(messages[imageIndex].source).toEqual({ ingress: 'acp', flavor: 'cursor' }); + } + clearGeneratedImages(); + }); }); diff --git a/cli/src/agent/backends/acp/AcpMessageHandler.ts b/cli/src/agent/backends/acp/AcpMessageHandler.ts index e3c01a61..a08fdb70 100644 --- a/cli/src/agent/backends/acp/AcpMessageHandler.ts +++ b/cli/src/agent/backends/acp/AcpMessageHandler.ts @@ -1,5 +1,8 @@ -import type { AgentMessage, PlanItem } from '@/agent/types'; import { randomUUID } from 'node:crypto'; +import { logger } from '@/ui/logger'; +import type { AgentMessage, PlanItem } from '@/agent/types'; +import { registerGeneratedImageFromAcpBlock } from '@/modules/common/generatedImages'; +import type { InlineMediaSource } from '@/modules/common/inlineMediaSource'; import { asString, isObject } from '@hapi/protocol'; import { deriveToolNameWithSource, isPlaceholderToolName } from '@/agent/utils'; import { parseRateLimitText } from '@/agent/rateLimitParser'; @@ -409,9 +412,9 @@ export class AcpMessageHandler { constructor( private readonly onMessage: (message: AgentMessage) => void, - options: { textChunkMode?: AcpTextChunkMode } = {} + private readonly options: { textChunkMode?: AcpTextChunkMode; flavor?: string } = {} ) { - this.textChunkMode = options.textChunkMode ?? 'dedupe'; + this.textChunkMode = this.options.textChunkMode ?? 'dedupe'; } /** @@ -577,7 +580,7 @@ export class AcpMessageHandler { this.reasoningSnapshotEmitted = false; } - handleUpdate(update: unknown): void { + async handleUpdate(update: unknown): Promise { if (!isObject(update)) return; const updateType = asString(update.sessionUpdate); if (!updateType) return; @@ -603,6 +606,12 @@ export class AcpMessageHandler { if (updateType === ACP_SESSION_UPDATE_TYPES.agentMessageChunk) { const content = update.content; + if (isObject(content) && content.type === 'image') { + this.flushReasoning(); + this.flushText(); + await this.emitGeneratedImageFromAcpContent(content); + return; + } const text = extractTextContent(content); if (text) { // Check once whether the buffered text is a prefix of this @@ -678,6 +687,35 @@ export class AcpMessageHandler { } } + private async emitGeneratedImageFromAcpContent(content: Record): Promise { + try { + const image = await registerGeneratedImageFromAcpBlock(content); + if (!image) { + return; + } + this.onMessage({ + type: 'generated_image', + imageId: image.id, + fileName: image.fileName, + mimeType: image.mimeType, + source: this.buildAcpInlineMediaSource(), + }); + } catch (error) { + logger.debug( + '[AcpMessageHandler] Failed to register ACP image block:', + error instanceof Error ? error.message : String(error) + ); + } + } + + private buildAcpInlineMediaSource(): InlineMediaSource { + const source: InlineMediaSource = { ingress: 'acp' }; + if (this.options?.flavor) { + source.flavor = this.options.flavor; + } + return source; + } + private handleToolCall(update: Record): void { const toolCallId = asString(update.toolCallId); if (!toolCallId) return; diff --git a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts index 4fcdb2de..ceabcc16 100644 --- a/cli/src/agent/backends/acp/AcpSdkBackend.test.ts +++ b/cli/src/agent/backends/acp/AcpSdkBackend.test.ts @@ -1165,6 +1165,7 @@ describe('AcpSdkBackend', () => { } | null; handleSessionUpdate: (params: unknown) => void; messageHandler: unknown; + sessionUpdateQueue: Promise; }; backendInternal.transport = { sendRequest: async () => ({ stopReason: 'end_turn' }), @@ -1194,12 +1195,16 @@ describe('AcpSdkBackend', () => { expect(result).toBe('compact result'); expect(handlerDuringSuppression).toBeNull(); + await backendInternal.sessionUpdateQueue; expect(turn1.some((m) => m.type === 'plan')).toBe(false); // The previous turn's handler must be back in place afterward so // ordinary straggler-forwarding (covered elsewhere) is unaffected. expect(backendInternal.messageHandler).toBe(handlerBeforeSuppression); emitPlanUpdate(); + // #958 queues message-handler work (async image registration); await + // before asserting delivery — upstream assumed sync handleUpdate. + await backendInternal.sessionUpdateQueue; expect(turn1.some((m) => m.type === 'plan')).toBe(true); }); @@ -1223,6 +1228,7 @@ describe('AcpSdkBackend', () => { } | null; handleSessionUpdate: (params: unknown) => void; messageHandler: unknown; + sessionUpdateQueue: Promise; }; backendInternal.transport = { sendRequest: async () => ({ stopReason: 'end_turn' }), @@ -1264,15 +1270,66 @@ describe('AcpSdkBackend', () => { expect(handlerDuringDrainWindow).toBeNull(); // Neither the immediate update nor the +15ms straggler leaked — // messageHandler was null (suppressed) for both. + await backendInternal.sessionUpdateQueue; expect(turn1.some((m) => m.type === 'plan')).toBe(false); expect(backendInternal.messageHandler).toBe(handlerBeforeSuppression); // Normal forwarding resumes once actually restored. emitPlanUpdate(); + await backendInternal.sessionUpdateQueue; expect(turn1.some((m) => m.type === 'plan')).toBe(true); }); + it('drops updates enqueued while suppressed even if the queue drains after the handler is restored', async () => { + // If handleSessionUpdate looked up this.messageHandler when the queued + // microtask ran (instead of capturing it at enqueue), a compact-era + // update stuck behind earlier async image work would leak into the + // restored handler after suppressUpdatesDuring returns. + backendStatics.PRE_PROMPT_UPDATE_QUIET_PERIOD_MS = 5; + backendStatics.PRE_PROMPT_UPDATE_DRAIN_TIMEOUT_MS = 50; + + const backend = new AcpSdkBackend({ command: 'opencode' }); + const backendInternal = backend as unknown as { + transport: { + sendRequest: (...args: unknown[]) => Promise; + close: () => Promise; + } | null; + handleSessionUpdate: (params: unknown) => void; + messageHandler: unknown; + sessionUpdateQueue: Promise; + }; + backendInternal.transport = { + sendRequest: async () => ({ stopReason: 'end_turn' }), + close: async () => {} + }; + + const turn1: AgentMessage[] = []; + await backend.prompt('session-1', [{ type: 'text', text: 'hi' }], (m) => turn1.push(m)); + + let releaseBlocker!: () => void; + const blocker = new Promise((resolve) => { + releaseBlocker = resolve; + }); + backendInternal.sessionUpdateQueue = backendInternal.sessionUpdateQueue.then(() => blocker); + + await backend.suppressUpdatesDuring(async () => { + backendInternal.handleSessionUpdate({ + sessionId: 'session-1', + update: { + sessionUpdate: ACP_SESSION_UPDATE_TYPES.plan, + entries: [{ content: 'queued-during-suppress', priority: 'medium', status: 'pending' }] + } + }); + return 'compact'; + }); + + expect(backendInternal.messageHandler).not.toBeNull(); + releaseBlocker(); + await backendInternal.sessionUpdateQueue; + expect(turn1.some((m) => m.type === 'plan')).toBe(false); + }); + it('does not let compact thought/text chunks escape through the next prompt pre-swap drain, while preserving the new prompt response', async () => { // The reported duplicate was not emitted during /compact itself. In // the pre-suppression implementation those chunks stayed in the old @@ -1293,6 +1350,7 @@ describe('AcpSdkBackend', () => { close: () => Promise; } | null; handleSessionUpdate: (params: unknown) => void; + sessionUpdateQueue: Promise; }; let promptRequestCount = 0; backendInternal.transport = { diff --git a/cli/src/agent/backends/acp/AcpSdkBackend.ts b/cli/src/agent/backends/acp/AcpSdkBackend.ts index f39a157d..a204463f 100644 --- a/cli/src/agent/backends/acp/AcpSdkBackend.ts +++ b/cli/src/agent/backends/acp/AcpSdkBackend.ts @@ -83,6 +83,7 @@ export class AcpSdkBackend implements AgentBackend { private usageUpdateListener: ((msg: AgentMessage) => void) | null = null; private sessionInfoUpdateListener: ((update: AcpSessionInfoUpdate) => void) | null = null; private lastForwardedUsageUpdate: AcpUsageUpdate | null = null; + private sessionUpdateQueue: Promise = Promise.resolve(); /** Retry configuration for ACP initialization */ private static readonly INIT_RETRY_OPTIONS = { @@ -120,6 +121,7 @@ export class AcpSdkBackend implements AgentBackend { args?: string[]; env?: Record; textChunkMode?: AcpTextChunkMode; + flavor?: AgentFlavor; }) {} async initialize(): Promise { @@ -509,8 +511,12 @@ export class AcpSdkBackend implements AgentBackend { AcpSdkBackend.PRE_PROMPT_UPDATE_QUIET_PERIOD_MS, AcpSdkBackend.PRE_PROMPT_UPDATE_DRAIN_TIMEOUT_MS ); + await this.sessionUpdateQueue; this.messageHandler?.drainBuffers(); - this.messageHandler = new AcpMessageHandler(onUpdate, { textChunkMode: this.options.textChunkMode }); + this.messageHandler = new AcpMessageHandler(onUpdate, { + textChunkMode: this.options.textChunkMode, + flavor: this.options.flavor, + }); this.isProcessingMessage = true; this.lastSessionUpdateAt = Date.now(); this.latestUsageUpdate = null; @@ -540,12 +546,17 @@ export class AcpSdkBackend implements AgentBackend { AcpSdkBackend.UPDATE_QUIET_PERIOD_MS, AcpSdkBackend.UPDATE_DRAIN_TIMEOUT_MS ); + await this.sessionUpdateQueue; this.messageHandler?.drainBuffers(); // Block here until the model truly stops streaming straggler // chunks (or LATE_FLUSH_WINDOW_MS elapses), so turn_complete and // the launcher's ready signal only fire once every chunk has been // emitted to this turn's onUpdate. await this.drainLateBuffers(); + // Late window can enqueue async image registration; drain again + // before turn_complete so generated_image precedes turn boundary. + await this.sessionUpdateQueue; + this.messageHandler?.drainBuffers(); try { const latestUsageUpdate = this.readLatestUsageUpdate(); if (promptUsage) { @@ -731,6 +742,7 @@ export class AcpSdkBackend implements AgentBackend { clearTimeout(timer); } this.sessionInfoRefreshTimers.clear(); + await this.sessionUpdateQueue; this.messageHandler?.drainBuffers(); this.messageHandler = null; this.activeSessionId = null; @@ -752,12 +764,28 @@ export class AcpSdkBackend implements AgentBackend { } this.lastSessionUpdateAt = Date.now(); const update = params.update; + // Title/usage/commands stay synchronous (#1028). Only message-handler + // work is queued so async image registration preserves event order. if (sessionId) { this.captureAvailableCommands(sessionId, update); } this.forwardSessionInfoUpdate(sessionId, update); this.captureUsageUpdate(update); - this.messageHandler?.handleUpdate(update); + // Capture the handler at enqueue time. Looking up `this.messageHandler` + // when the queued microtask runs can leak a suppressUpdatesDuring + // update into the restored handler if earlier async image work kept + // the queue busy past restore. + const handler = this.messageHandler; + this.sessionUpdateQueue = this.sessionUpdateQueue + .then(async () => { + await handler?.handleUpdate(update); + }) + .catch((error) => { + logger.debug( + '[AcpSdkBackend] session update failed:', + error instanceof Error ? error.message : String(error) + ); + }); } private forwardSessionInfoUpdate(sessionId: string | null, update: unknown): void { diff --git a/cli/src/agent/messageConverter.test.ts b/cli/src/agent/messageConverter.test.ts index 83db6be9..ac11db6d 100644 --- a/cli/src/agent/messageConverter.test.ts +++ b/cli/src/agent/messageConverter.test.ts @@ -213,4 +213,23 @@ describe('convertAgentMessage', () => { // be fail-closed. expect(convertAgentMessage({ type: 'not_a_real_type' } as never)).toBeNull(); }); + + it('converts generated_image messages into generated-image wire payloads', () => { + const converted = convertAgentMessage({ + type: 'generated_image', + imageId: 'img-1', + fileName: 'inline.png', + mimeType: 'image/png', + source: { ingress: 'mcp', toolName: 'display_image' }, + }); + + expect(converted).toMatchObject({ + type: 'generated-image', + imageId: 'img-1', + fileName: 'inline.png', + mimeType: 'image/png', + source: { ingress: 'mcp', toolName: 'display_image' }, + }); + expect(converted && 'id' in converted && typeof converted.id === 'string').toBe(true); + }); }); diff --git a/cli/src/agent/messageConverter.ts b/cli/src/agent/messageConverter.ts index 4df39570..1c7699b5 100644 --- a/cli/src/agent/messageConverter.ts +++ b/cli/src/agent/messageConverter.ts @@ -1,6 +1,7 @@ import { randomUUID } from 'node:crypto'; import { INCLUSIVE_INPUT_TOKEN_USAGE_MARKER, type InclusiveInputTokenUsageMarker } from '@hapi/protocol/usage'; import type { AgentMessage, PlanItem } from './types'; +import type { InlineMediaSource } from '@/modules/common/inlineMediaSource'; export type CodexMessage = | { type: 'message'; message: string; id?: string; streamSnapshot?: boolean } @@ -40,7 +41,15 @@ export type CodexMessage = is_error?: boolean; } | { type: 'plan'; entries: PlanItem[] } - | { type: 'error'; message: string }; + | { type: 'error'; message: string } + | { + type: 'generated-image'; + imageId: string; + fileName: string; + mimeType: string; + id: string; + source?: InlineMediaSource; + }; export function convertAgentMessage(message: AgentMessage, model?: string | null): CodexMessage | null { switch (message.type) { @@ -101,6 +110,15 @@ export function convertAgentMessage(message: AgentMessage, model?: string | null type: 'plan', entries: message.items }; + case 'generated_image': + return { + type: 'generated-image', + imageId: message.imageId, + fileName: message.fileName, + mimeType: message.mimeType, + id: randomUUID(), + source: message.source, + }; case 'error': return { type: 'error', message: message.message }; case 'turn_complete': diff --git a/cli/src/agent/runners/runAgentSession.test.ts b/cli/src/agent/runners/runAgentSession.test.ts index d69f66b5..fe188657 100644 --- a/cli/src/agent/runners/runAgentSession.test.ts +++ b/cli/src/agent/runners/runAgentSession.test.ts @@ -74,7 +74,7 @@ vi.mock('@/claude/utils/startHappyServer', () => ({ harness.startHappyServerOptions = options return { url: 'http://127.0.0.1:1234', - toolNames: ['change_title', 'display_image', 'list_peers', 'ping_peer', 'inspect_peer', 'skill_lookup'], + toolNames: ['change_title', 'display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer', 'skill_lookup'], stop: harness.stopServer } }) @@ -167,7 +167,7 @@ describe('runAgentSession', () => { '--url', 'http://127.0.0.1:1234', '--tools', - 'change_title,display_image,list_peers,ping_peer,inspect_peer,skill_lookup' + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup' ]) expect(harness.newSessionOptions).toMatchObject({ cwd: '/tmp/project', diff --git a/cli/src/agent/types.ts b/cli/src/agent/types.ts index 65e849d4..6a394946 100644 --- a/cli/src/agent/types.ts +++ b/cli/src/agent/types.ts @@ -1,4 +1,5 @@ import type { AgentFlavor } from '@hapi/protocol'; +import type { InlineMediaSource } from '@/modules/common/inlineMediaSource'; export type McpEnvVar = { name: string; @@ -54,6 +55,7 @@ export type AgentMessage = contextWindow?: number; } | { type: 'plan'; items: PlanItem[] } + | { type: 'generated_image'; imageId: string; fileName: string; mimeType: string; source?: InlineMediaSource } | { type: 'turn_complete'; stopReason: string } | { type: 'error'; message: string }; diff --git a/cli/src/claude/utils/startHappyServer.test.ts b/cli/src/claude/utils/startHappyServer.test.ts index 3e47fa0c..d7f0e70a 100644 --- a/cli/src/claude/utils/startHappyServer.test.ts +++ b/cli/src/claude/utils/startHappyServer.test.ts @@ -18,6 +18,7 @@ describe('startHappyServer skill_lookup', () => { let workingDirectory: string let client: Client | null let stopServer: (() => void) | null + let sendAgentMessage: ReturnType beforeEach(async () => { sandboxDir = await mkdtemp(join(tmpdir(), 'hapi-skill-mcp-')) @@ -41,9 +42,10 @@ describe('startHappyServer skill_lookup', () => { }) async function connect(enableSkillLookup = true): Promise { + sendAgentMessage = vi.fn() const sessionClient = { updateMetadata: vi.fn(), - sendAgentMessage: vi.fn(), + sendAgentMessage, sendClaudeSessionMessage: vi.fn() } as unknown as ApiSessionClient const server = await startHappyServer(sessionClient, enableSkillLookup @@ -108,12 +110,34 @@ describe('startHappyServer skill_lookup', () => { expect(tools.tools.map((tool) => tool.name)).toEqual([ 'change_title', 'display_image', + 'display_video', + 'display_media', 'ping_peer', 'inspect_peer', 'list_peers' ]) }) + it('displays audio through display_media and emits a generated media message', async () => { + const path = join(sandboxDir, 'sample.wav') + await writeFile(path, Buffer.from('RIFFxxxxWAVE')) + const mcp = await connect(false) + + const result = await mcp.callTool({ + name: 'display_media', + arguments: { path, title: 'sample.wav' } + }) as ToolResult + + expect(result.isError).toBe(false) + expect(result.content?.[0]?.text).toContain('Displayed media: sample.wav') + expect(sendAgentMessage).toHaveBeenCalledWith(expect.objectContaining({ + type: 'generated-image', + fileName: 'sample.wav', + mimeType: 'audio/wav', + source: { ingress: 'mcp', toolName: 'display_media' } + })) + }) + it('does not expose change_title when native ACP titles are enabled', async () => { const sessionClient = { updateMetadata: vi.fn(), @@ -128,9 +152,11 @@ describe('startHappyServer skill_lookup', () => { await mcp.connect(new StreamableHTTPClientTransport(new URL(server.url))) const tools = await mcp.listTools() - expect(server.toolNames).toEqual(['display_image', 'list_peers', 'ping_peer', 'inspect_peer']) + expect(server.toolNames).toEqual(['display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer']) expect(tools.tools.map((tool) => tool.name)).toEqual([ 'display_image', + 'display_video', + 'display_media', 'ping_peer', 'inspect_peer', 'list_peers' @@ -140,10 +166,12 @@ describe('startHappyServer skill_lookup', () => { }) describe('toClaudeAllowedHapiMcpTools', () => { - it('keeps ping_peer and inspect_peer registered but out of Claude --allowedTools', () => { + it('keeps local-path and peer tools registered but out of Claude --allowedTools', () => { expect(toClaudeAllowedHapiMcpTools([ 'change_title', 'display_image', + 'display_video', + 'display_media', 'list_peers', 'ping_peer', 'inspect_peer', @@ -154,5 +182,7 @@ describe('toClaudeAllowedHapiMcpTools', () => { 'mcp__hapi__list_peers', 'mcp__hapi__skill_lookup' ]) + expect(toClaudeAllowedHapiMcpTools(['display_video'])).not.toContain('mcp__hapi__display_video') + expect(toClaudeAllowedHapiMcpTools(['display_media'])).not.toContain('mcp__hapi__display_media') }) }) diff --git a/cli/src/claude/utils/startHappyServer.ts b/cli/src/claude/utils/startHappyServer.ts index f8d8e73d..6060bdc2 100644 --- a/cli/src/claude/utils/startHappyServer.ts +++ b/cli/src/claude/utils/startHappyServer.ts @@ -5,14 +5,21 @@ import { McpServer } from "@modelcontextprotocol/sdk/server/mcp.js"; import { createServer, type IncomingMessage } from "node:http"; -import { lstat, readFile } from "node:fs/promises"; import { StreamableHTTPServerTransport } from "@modelcontextprotocol/sdk/server/streamableHttp.js"; import { AddressInfo } from "node:net"; import { z } from "zod"; import { logger } from "@/ui/logger"; import { ApiSessionClient } from "@/api/apiSession"; import { randomUUID } from "node:crypto"; -import { detectImageMimeType, registerGeneratedImage } from "@/modules/common/generatedImages"; +import { + detectDisplayMediaMimeType, + detectImageMimeType, + detectVideoMimeType, + readBoundedRegularFile, + registerGeneratedImage, +} from "@/modules/common/generatedImages"; +import type { InlineMediaSource } from "@/modules/common/inlineMediaSource"; +import { DISPLAY_IMAGE_PROMPT_CURSOR, DISPLAY_MEDIA_PROMPT_CURSOR, DISPLAY_VIDEO_PROMPT_CURSOR } from "@/modules/common/displayImagePrompt"; import { resolveSkill } from "@/modules/common/skills"; import { INSPECT_PEER_TOOL_DESCRIPTION, @@ -31,12 +38,17 @@ type StartHappyServerOptions = { }; /** Registered on the MCP server, but never pre-approved via Claude --allowedTools. */ -const CLAUDE_MANUAL_APPROVAL_HAPI_TOOLS = new Set(['ping_peer', 'inspect_peer']); +const CLAUDE_MANUAL_APPROVAL_HAPI_TOOLS = new Set([ + 'display_media', + 'display_video', + 'ping_peer', + 'inspect_peer' +]); /** * Map HAPI MCP tool names to Claude `--allowedTools` entries. - * Keeps `ping_peer` / `inspect_peer` off the auto-allow list so cross-session - * write (resume+inject) and read (peer histories) still prompt. + * Keeps `display_media` / `display_video` (arbitrary local-path readers), `ping_peer`, and + * `inspect_peer` off the auto-allow list so they still prompt. * `list_peers` stays allowed (discovery shortlist only). */ export function toClaudeAllowedHapiMcpTools(toolNames: string[]): string[] { @@ -82,11 +94,27 @@ function createHapiMcpServer( title: z.string().optional().describe('Optional display title or filename for the image'), }); + const skillLookupInputSchema: z.ZodTypeAny = z.object({ + name: z.string().trim().min(1).max(128).describe('Exact skill name shown by HAPI skill autocomplete'), + }); + + const displayVideoInputSchema: z.ZodTypeAny = z.object({ + path: z.string().describe('Local filesystem path of the video to display inline (mp4 or webm)'), + title: z.string().optional().describe('Optional display title or filename for the video'), + }); + + const displayMediaInputSchema: z.ZodTypeAny = z.object({ + path: z.string().describe('Local filesystem path of the media or file to send to the user'), + title: z.string().trim().min(1).max(255).optional().describe('Optional display title or filename'), + }); + const pingPeerInputSchema: z.ZodTypeAny = z.object({ sessionIdPrefix: z.string().trim().min(1).describe(SESSION_ID_PREFIX_PARAM_DESCRIPTION), message: z.string().min(1).describe('Message text to deliver to the target session'), }); + const maxInlineMediaBytes = 25 * 1024 * 1024; + const inspectPeerInputSchema: z.ZodTypeAny = z.object({ sessionIdPrefix: z.string().trim().min(1).describe(SESSION_ID_PREFIX_PARAM_DESCRIPTION), messageLimit: z.number().int().min(1).max(100).optional().describe( @@ -100,13 +128,48 @@ function createHapiMcpServer( ), }); - const skillLookupInputSchema: z.ZodTypeAny = z.object({ - name: z.string().trim().min(1).max(128).describe('Exact skill name shown by HAPI skill autocomplete'), - }); + async function displayInlineMedia( + args: { path: string; title?: string }, + mediaKind: 'image' | 'video' | 'media', + toolName: 'display_image' | 'display_video' | 'display_media' + ) { + const bytes = await readBoundedRegularFile(args.path, maxInlineMediaBytes); + const mimeType = mediaKind === 'video' + ? detectVideoMimeType(bytes) + : mediaKind === 'image' + ? detectImageMimeType(bytes) + : detectDisplayMediaMimeType(bytes); + if (!mimeType) { + throw new Error(mediaKind === 'video' ? 'Unsupported video content' : 'Unsupported image content'); + } + const media = registerGeneratedImage({ + id: randomUUID(), + path: args.path, + fileName: args.title, + mimeType, + bytes + }); + + const source: InlineMediaSource = { + ingress: 'mcp', + toolName, + }; + + client.sendAgentMessage({ + type: 'generated-image', + imageId: media.id, + fileName: media.fileName, + mimeType: media.mimeType, + id: randomUUID(), + source, + }); + + return media; + } if (enableChangeTitle) { mcp.registerTool('change_title', { - description: 'Change the title of the current chat session', + description: 'Change the title of the current HAPI chat session. Call once when the user\'s primary objective is clear; use a concise task title.', title: 'Change Chat Title', inputSchema: changeTitleInputSchema, }, async (args: { title: string }) => { @@ -138,44 +201,14 @@ function createHapiMcpServer( } mcp.registerTool('display_image', { - description: 'Display a local image file inline in the current HAPI chat session', + description: `Display a local image file inline in the current HAPI chat session. ${DISPLAY_IMAGE_PROMPT_CURSOR}`, title: 'Display Image', inputSchema: displayImageInputSchema, }, async (args: { path: string; title?: string }) => { logger.debug('[hapiMCP] Display image:', args.path); try { - const info = await lstat(args.path); - if (!info.isFile()) { - throw new Error('Path is not a regular file'); - } - - const maxImageBytes = 25 * 1024 * 1024; - if (info.size > maxImageBytes) { - throw new Error('Image is too large to display inline'); - } - - const bytes = await readFile(args.path); - const mimeType = detectImageMimeType(bytes); - if (!mimeType) { - throw new Error('Unsupported image content'); - } - - const image = registerGeneratedImage({ - id: randomUUID(), - path: args.path, - fileName: args.title, - mimeType, - bytes - }); - - client.sendAgentMessage({ - type: 'generated-image', - imageId: image.id, - fileName: image.fileName, - mimeType: image.mimeType, - id: randomUUID() - }); + const image = await displayInlineMedia(args, 'image', 'display_image'); return { content: [ @@ -201,6 +234,63 @@ function createHapiMcpServer( } }); + mcp.registerTool('display_video', { + description: `Display a local mp4 or webm file inline in the current HAPI chat session. ${DISPLAY_VIDEO_PROMPT_CURSOR}`, + title: 'Display Video', + inputSchema: displayVideoInputSchema, + }, async (args: { path: string; title?: string }) => { + logger.debug('[hapiMCP] Display video:', args.path); + + try { + const video = await displayInlineMedia(args, 'video', 'display_video'); + + return { + content: [ + { + type: 'text' as const, + text: `Displayed video: ${video.fileName}`, + }, + ], + isError: false, + }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.debug('[hapiMCP] Failed to display video:', message); + return { + content: [ + { + type: 'text' as const, + text: `Failed to display video: ${message}`, + }, + ], + isError: true, + }; + } + }); + + mcp.registerTool('display_media', { + description: `Send a local image, video, audio, or other file to the current HAPI chat session. Recognized media is shown inline; other files use a download card. ${DISPLAY_MEDIA_PROMPT_CURSOR}`, + title: 'Display Media', + inputSchema: displayMediaInputSchema, + }, async (args: { path: string; title?: string }) => { + logger.debug('[hapiMCP] Display media:', args.path); + + try { + const media = await displayInlineMedia(args, 'media', 'display_media'); + return { + content: [{ type: 'text' as const, text: `Displayed media: ${media.fileName}` }], + isError: false, + }; + } catch (error) { + const message = error instanceof Error ? error.message : String(error); + logger.debug('[hapiMCP] Failed to display media:', message); + return { + content: [{ type: 'text' as const, text: `Failed to display media: ${message}` }], + isError: true, + }; + } + }); + mcp.registerTool('ping_peer', { description: PING_PEER_TOOL_DESCRIPTION, title: 'Ping Peer Session', @@ -323,6 +413,7 @@ function createHapiMcpServer( } }); + if (skillLookup) { mcp.registerTool('skill_lookup', { description: 'Load a HAPI skill by exact name. When a user message starts with $name, call this tool with that name before acting.', @@ -443,8 +534,8 @@ export async function startHappyServer(client: ApiSessionClient, options: StartH })); const toolNames = enableChangeTitle - ? ['change_title', 'display_image', 'list_peers', 'ping_peer', 'inspect_peer'] - : ['display_image', 'list_peers', 'ping_peer', 'inspect_peer']; + ? ['change_title', 'display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer'] + : ['display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer']; if (options.skillLookup) { toolNames.push('skill_lookup'); } diff --git a/cli/src/claude/utils/systemPrompt.ts b/cli/src/claude/utils/systemPrompt.ts index 7462008a..3174ac6e 100644 --- a/cli/src/claude/utils/systemPrompt.ts +++ b/cli/src/claude/utils/systemPrompt.ts @@ -1,6 +1,7 @@ import { trimIdent } from "@/utils/trimIdent"; import { buildSessionCitationSteerInstruction } from "@hapi/protocol/sessionCitation"; import { shouldIncludeCoAuthoredBy } from "./claudeSettings"; +import { DISPLAY_IMAGE_PROMPT_CLAUDE, DISPLAY_MEDIA_PROMPT_CLAUDE, DISPLAY_VIDEO_PROMPT_CLAUDE } from "@/modules/common/displayImagePrompt"; import { withSessionSummaryInstruction } from "@/modules/common/sessionSummaryInstruction"; /** @@ -8,7 +9,9 @@ import { withSessionSummaryInstruction } from "@/modules/common/sessionSummaryIn */ const BASE_SYSTEM_PROMPT = (() => trimIdent(` Use the title tool sparingly. For a new chat, call the tool "mcp__hapi__change_title" once after the user's initial request is clear, and set a concise task title. Do not rename the chat for routine progress, substeps, implementation details, or a slightly better wording. Rename only when the user's primary objective changes substantially and the existing title would be misleading. - When you create or find a local image file that the user should see, call the tool "mcp__hapi__display_image" with the image path so HAPI can show it inline. + ${DISPLAY_IMAGE_PROMPT_CLAUDE} + ${DISPLAY_VIDEO_PROMPT_CLAUDE} + ${DISPLAY_MEDIA_PROMPT_CLAUDE} ${buildSessionCitationSteerInstruction({ inspectTool: 'mcp__hapi__inspect_peer', pingTool: 'mcp__hapi__ping_peer', diff --git a/cli/src/codex/codexRemoteLauncher.ts b/cli/src/codex/codexRemoteLauncher.ts index 9436db75..aee33e3c 100644 --- a/cli/src/codex/codexRemoteLauncher.ts +++ b/cli/src/codex/codexRemoteLauncher.ts @@ -1,6 +1,5 @@ import React from 'react'; import { randomUUID } from 'node:crypto'; -import { lstat, readFile } from 'node:fs/promises'; import { CodexAppServerClient } from './codexAppServerClient'; import { CodexPermissionHandler } from './utils/permissionHandler'; @@ -14,7 +13,7 @@ import type { CodexSession } from './session'; import type { EnhancedMode } from './loop'; import { hasCodexCliOverrides } from './utils/codexCliOverrides'; import { AppServerEventConverter } from './utils/appServerEventConverter'; -import { detectImageMimeType, registerGeneratedImage } from '@/modules/common/generatedImages'; +import { registerGeneratedImageFromPath } from '@/modules/common/generatedImages'; import { registerAppServerPermissionHandlers } from './utils/appServerPermissionAdapter'; import { buildThreadStartParams, buildTurnStartParams } from './utils/appServerConfig'; import type { SkillMetadata, ThreadGoal, ThreadGoalStatus } from './appServerTypes'; @@ -30,32 +29,17 @@ import { import { CodexConversationHistory } from './conversationHistory'; -async function registerGeneratedImageFromPath(args: { id: string; path: string; fileName?: string | null }): Promise | null> { - try { - const info = await lstat(args.path); - if (!info.isFile()) { - throw new Error('Path is not a regular file'); - } - const maxImageBytes = 25 * 1024 * 1024; - if (info.size > maxImageBytes) { - throw new Error('Image is too large to display inline'); - } - const bytes = await readFile(args.path); - const mimeType = detectImageMimeType(bytes); - if (!mimeType) { - throw new Error('Unsupported image content'); - } - return registerGeneratedImage({ - id: args.id, - path: args.path, - fileName: args.fileName, - mimeType, - bytes - }); - } catch (error) { - logger.debug('[CodexRemoteLauncher] Failed to register generated image:', error instanceof Error ? error.message : String(error)); - return null; + +async function registerGeneratedImageFromPathWrapper(args: { id: string; path: string; fileName?: string | null }): Promise> | null> { + const image = await registerGeneratedImageFromPath({ + id: args.id, + path: args.path, + fileName: args.fileName + }); + if (!image) { + logger.debug('[CodexRemoteLauncher] Failed to register generated image from path'); } + return image; } type HappyServer = Awaited>['server']; @@ -2834,7 +2818,7 @@ class CodexRemoteLauncher extends RemoteLauncherBase { const imageId = randomUUID(); const savedPath = asString(msg.saved_path ?? msg.savedPath); if (savedPath) { - const image = await registerGeneratedImageFromPath({ + const image = await registerGeneratedImageFromPathWrapper({ id: imageId, path: savedPath, fileName: asString(msg.file_name ?? msg.fileName) @@ -2848,7 +2832,12 @@ class CodexRemoteLauncher extends RemoteLauncherBase { sourceImageId, fileName: image.fileName, mimeType: image.mimeType, - id: randomUUID() + id: randomUUID(), + source: { + ingress: 'tool_result', + flavor: 'codex', + toolCallId: asString(msg.call_id ?? msg.callId), + }, }); } } diff --git a/cli/src/codex/happyMcpStdioBridge.test.ts b/cli/src/codex/happyMcpStdioBridge.test.ts index 38972d14..48d1c6a2 100644 --- a/cli/src/codex/happyMcpStdioBridge.test.ts +++ b/cli/src/codex/happyMcpStdioBridge.test.ts @@ -53,15 +53,14 @@ describe('runHappyMcpStdioBridge tool forwarding', () => { '--url', 'http://127.0.0.1:43006', '--tools', - 'change_title,display_image,list_peers,ping_peer,inspect_peer,skill_lookup' + 'change_title,display_image,display_video,display_media,skill_lookup' ]) expect([...harness.tools.keys()]).toEqual([ 'change_title', 'display_image', - 'ping_peer', - 'inspect_peer', - 'list_peers', + 'display_video', + 'display_media', 'skill_lookup' ]) @@ -82,15 +81,82 @@ describe('runHappyMcpStdioBridge tool forwarding', () => { '--url', 'http://127.0.0.1:43006', '--tools', - 'change_title,display_image,list_peers,ping_peer,inspect_peer' + 'change_title,display_image,display_video' + ]) + + expect([...harness.tools.keys()]).toEqual(['change_title', 'display_image', 'display_video']) + }) + + it('forwards display_media arguments unchanged', async () => { + await runHappyMcpStdioBridge([ + '--url', + 'http://127.0.0.1:43006', + '--tools', + 'display_media' + ]) + + const handler = harness.tools.get('display_media') + await expect(handler?.({ path: '/tmp/sample.wav', title: 'sample.wav' })).resolves.toEqual({ + content: [{ type: 'text', text: 'forwarded' }], + isError: false + }) + expect(harness.callTool).toHaveBeenCalledWith({ + name: 'display_media', + arguments: { path: '/tmp/sample.wav', title: 'sample.wav' } + }) + }) + + it('registers ping_peer when included in --tools', async () => { + await runHappyMcpStdioBridge([ + '--url', + 'http://127.0.0.1:43006', + '--tools', + 'change_title,display_image,display_video,display_media,ping_peer' ]) expect([...harness.tools.keys()]).toEqual([ 'change_title', 'display_image', - 'ping_peer', - 'inspect_peer', - 'list_peers' + 'display_video', + 'display_media', + 'ping_peer' ]) }) + + it('registers inspect_peer when included in --tools', async () => { + await runHappyMcpStdioBridge([ + '--url', + 'http://127.0.0.1:43006', + '--tools', + 'change_title,display_image,display_video,display_media,ping_peer,inspect_peer' + ]) + + expect([...harness.tools.keys()]).toEqual([ + 'change_title', + 'display_image', + 'display_video', + 'display_media', + 'ping_peer', + 'inspect_peer' + ]) + }) + it('registers list_peers when included in --tools', async () => { + await runHappyMcpStdioBridge([ + '--url', + 'http://127.0.0.1:43006', + '--tools', + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer' + ]) + + expect([...harness.tools.keys()]).toEqual([ + 'change_title', + 'display_image', + 'display_video', + 'display_media', + 'ping_peer', + 'inspect_peer', + 'list_peers', + ]) + }) + }) diff --git a/cli/src/codex/happyMcpStdioBridge.ts b/cli/src/codex/happyMcpStdioBridge.ts index f43e2b6c..242fd95f 100644 --- a/cli/src/codex/happyMcpStdioBridge.ts +++ b/cli/src/codex/happyMcpStdioBridge.ts @@ -1,7 +1,7 @@ /** * HAPI MCP STDIO Bridge * - * Minimal STDIO MCP server exposing HAPI tools such as `change_title`, `display_image`, and `ping_peer`. + * Minimal STDIO MCP server exposing HAPI tools such as `change_title`, `display_image`, `display_video`, `display_media`, `list_peers`, `ping_peer`, and `inspect_peer`. * On invocation it forwards the tool call to an existing HAPI HTTP MCP server * using the StreamableHTTPClientTransport. * @@ -16,13 +16,14 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { Client } from '@modelcontextprotocol/sdk/client/index.js'; import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js'; import { z } from 'zod'; +import { DISPLAY_IMAGE_PROMPT_CURSOR, DISPLAY_MEDIA_PROMPT_CURSOR, DISPLAY_VIDEO_PROMPT_CURSOR } from '@/modules/common/displayImagePrompt'; import { INSPECT_PEER_TOOL_DESCRIPTION, PING_PEER_TOOL_DESCRIPTION, SESSION_ID_PREFIX_PARAM_DESCRIPTION, } from '@hapi/protocol/sessionCitation'; -const DEFAULT_TOOL_NAMES = ['change_title', 'display_image', 'list_peers', 'ping_peer', 'inspect_peer']; +const DEFAULT_TOOL_NAMES = ['change_title', 'display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer']; function parseArgs(argv: string[]): { url: string | null; toolNames: Set } { let url: string | null = null; @@ -117,7 +118,7 @@ export async function runHappyMcpStdioBridge(argv: string[]): Promise { server.registerTool( 'display_image', { - description: 'Display a local image file inline in the current HAPI chat session', + description: `Display a local image file inline in the current HAPI chat session. ${DISPLAY_IMAGE_PROMPT_CURSOR}`, title: 'Display Image', inputSchema: displayImageInputSchema, }, @@ -138,6 +139,63 @@ export async function runHappyMcpStdioBridge(argv: string[]): Promise { ); } + const displayVideoInputSchema: z.ZodTypeAny = z.object({ + path: z.string().describe('Local filesystem path of the video to display inline (mp4 or webm)'), + title: z.string().optional().describe('Optional display title or filename for the video'), + }); + + if (toolNames.has('display_video')) { + server.registerTool( + 'display_video', + { + description: `Display a local mp4 or webm file inline in the current HAPI chat session. ${DISPLAY_VIDEO_PROMPT_CURSOR}`, + title: 'Display Video', + inputSchema: displayVideoInputSchema, + }, + async (args: Record) => { + try { + const client = await ensureHttpClient(); + const response = await client.callTool({ name: 'display_video', arguments: args }); + return response as any; + } catch (error) { + return { + content: [ + { type: 'text' as const, text: `Failed to display video: ${error instanceof Error ? error.message : String(error)}` }, + ], + isError: true, + }; + } + } + ); + } + + const displayMediaInputSchema: z.ZodTypeAny = z.object({ + path: z.string().describe('Local filesystem path of the media or file to send to the user'), + title: z.string().trim().min(1).max(255).optional().describe('Optional display title or filename'), + }); + + if (toolNames.has('display_media')) { + server.registerTool( + 'display_media', + { + description: `Send a local image, video, audio, or other file to the current HAPI chat session. ${DISPLAY_MEDIA_PROMPT_CURSOR}`, + title: 'Display Media', + inputSchema: displayMediaInputSchema, + }, + async (args: Record) => { + try { + const client = await ensureHttpClient(); + return await client.callTool({ name: 'display_media', arguments: args }) as any; + } catch (error) { + return { + content: [{ type: 'text' as const, text: `Failed to display media: ${error instanceof Error ? error.message : String(error)}` }], + isError: true, + }; + } + } + ); + } + const pingPeerInputSchema: z.ZodTypeAny = z.object({ sessionIdPrefix: z.string().trim().min(1).describe(SESSION_ID_PREFIX_PARAM_DESCRIPTION), message: z.string().min(1).describe('Message text to deliver to the target session'), diff --git a/cli/src/codex/utils/buildHapiMcpBridge.test.ts b/cli/src/codex/utils/buildHapiMcpBridge.test.ts index d7bdc007..8af0d577 100644 --- a/cli/src/codex/utils/buildHapiMcpBridge.test.ts +++ b/cli/src/codex/utils/buildHapiMcpBridge.test.ts @@ -14,8 +14,8 @@ vi.mock('@/claude/utils/startHappyServer', () => ({ return { url: 'http://127.0.0.1:43006/', toolNames: options.skillLookup - ? ['change_title', 'display_image', 'list_peers', 'ping_peer', 'inspect_peer', 'skill_lookup'] - : ['change_title', 'display_image', 'list_peers', 'ping_peer', 'inspect_peer'], + ? ['change_title', 'display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer', 'skill_lookup'] + : ['change_title', 'display_image', 'display_video', 'display_media', 'list_peers', 'ping_peer', 'inspect_peer'], stop: vi.fn() } }) @@ -71,10 +71,13 @@ describe('buildHapiMcpBridge skill lookup config', () => { '--url', 'http://127.0.0.1:43006/', '--tools', - 'change_title,display_image,list_peers,ping_peer,inspect_peer,skill_lookup' + 'change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer,skill_lookup' ]) expect(bridge.mcpServers.hapi.tools).toEqual({ change_title: { approval_mode: 'approve' }, + display_image: { approval_mode: 'prompt' }, + display_video: { approval_mode: 'prompt' }, + display_media: { approval_mode: 'prompt' }, list_peers: { approval_mode: 'approve' }, skill_lookup: { approval_mode: 'approve' } }) @@ -83,9 +86,12 @@ describe('buildHapiMcpBridge skill lookup config', () => { it('does not expose skill_lookup for native-skill bridge callers', async () => { const bridge = await buildHapiMcpBridge(createClient()) - expect(harness.cliArgs.at(-1)).toBe('change_title,display_image,list_peers,ping_peer,inspect_peer') + expect(harness.cliArgs.at(-1)).toBe('change_title,display_image,display_video,display_media,list_peers,ping_peer,inspect_peer') expect(bridge.mcpServers.hapi.tools).toEqual({ change_title: { approval_mode: 'approve' }, + display_image: { approval_mode: 'prompt' }, + display_video: { approval_mode: 'prompt' }, + display_media: { approval_mode: 'prompt' }, list_peers: { approval_mode: 'approve' } }) }) diff --git a/cli/src/codex/utils/buildHapiMcpBridge.ts b/cli/src/codex/utils/buildHapiMcpBridge.ts index a4b370b8..f92621f7 100644 --- a/cli/src/codex/utils/buildHapiMcpBridge.ts +++ b/cli/src/codex/utils/buildHapiMcpBridge.ts @@ -89,7 +89,17 @@ export async function buildHapiMcpBridge( '--tools', happyServer.toolNames.join(',') ]); - const tools: Record = {}; + const tools: Record = { + display_image: { + approval_mode: 'prompt' + }, + display_video: { + approval_mode: 'prompt' + }, + display_media: { + approval_mode: 'prompt' + } + }; if (options.enableChangeTitle !== false) { tools.change_title = { approval_mode: 'approve' diff --git a/cli/src/codex/utils/codexMcpConfig.test.ts b/cli/src/codex/utils/codexMcpConfig.test.ts index f094fffd..7cc6070e 100644 --- a/cli/src/codex/utils/codexMcpConfig.test.ts +++ b/cli/src/codex/utils/codexMcpConfig.test.ts @@ -31,6 +31,15 @@ describe('codexMcpConfig', () => { tools: { change_title: { approval_mode: 'approve' as const + }, + display_image: { + approval_mode: 'prompt' as const + }, + display_video: { + approval_mode: 'prompt' as const + }, + display_media: { + approval_mode: 'prompt' as const } } } @@ -39,6 +48,9 @@ describe('codexMcpConfig', () => { const args = buildMcpServerConfigArgs(mcpServers); expect(args).toContain('mcp_servers.hapi.tools.change_title.approval_mode="approve"'); + expect(args).toContain('mcp_servers.hapi.tools.display_image.approval_mode="prompt"'); + expect(args).toContain('mcp_servers.hapi.tools.display_video.approval_mode="prompt"'); + expect(args).toContain('mcp_servers.hapi.tools.display_media.approval_mode="prompt"'); }); it('builds config args for multiple MCP servers', () => { diff --git a/cli/src/codex/utils/systemPrompt.ts b/cli/src/codex/utils/systemPrompt.ts index f51ab80f..bd85efc3 100644 --- a/cli/src/codex/utils/systemPrompt.ts +++ b/cli/src/codex/utils/systemPrompt.ts @@ -7,6 +7,7 @@ import { trimIdent } from '@/utils/trimIdent'; import { buildSessionCitationSteerInstruction } from '@hapi/protocol/sessionCitation'; +import { DISPLAY_IMAGE_PROMPT_CODEX, DISPLAY_MEDIA_PROMPT_CODEX, DISPLAY_VIDEO_PROMPT_CODEX } from '@/modules/common/displayImagePrompt'; import { withSessionSummaryInstruction } from '@/modules/common/sessionSummaryInstruction'; /** @@ -20,7 +21,9 @@ export const TITLE_INSTRUCTION = trimIdent(` If that exact tool name is unavailable, call an equivalent alias such as hapi__change_title, mcp__hapi__change_title, or hapi_change_title. Do not rename the chat for routine progress, substeps, implementation details, or a slightly better wording. Rename only when the user's primary objective changes substantially and the existing title would be misleading. - When you create or find a local image file that the user should see, call functions.hapi__display_image with the image path. If that exact tool name is unavailable, use an equivalent alias such as hapi__display_image, mcp__hapi__display_image, or hapi_display_image. + ${DISPLAY_IMAGE_PROMPT_CODEX} + ${DISPLAY_VIDEO_PROMPT_CODEX} + ${DISPLAY_MEDIA_PROMPT_CODEX} ${buildSessionCitationSteerInstruction({ inspectTool: 'functions.hapi__inspect_peer', pingTool: 'functions.hapi__ping_peer', diff --git a/cli/src/commands/doctor.ts b/cli/src/commands/doctor.ts index 39067ae9..17fab174 100644 --- a/cli/src/commands/doctor.ts +++ b/cli/src/commands/doctor.ts @@ -1,5 +1,6 @@ import { killRunawayHappyProcesses } from '@/runner/doctor' import { runDoctorCommand } from '@/ui/doctor' +import { runDoctorInlineMedia } from '@/ui/doctorInlineMedia' import type { CommandDefinition } from './types' export const doctorCommand: CommandDefinition = { @@ -14,6 +15,10 @@ export const doctorCommand: CommandDefinition = { } process.exit(0) } + if (commandArgs[0] === 'inline-media') { + const code = await runDoctorInlineMedia() + process.exit(code) + } await runDoctorCommand() } } diff --git a/cli/src/copilot/copilotRemoteLauncher.ts b/cli/src/copilot/copilotRemoteLauncher.ts index 8b06ecbe..48300c1a 100644 --- a/cli/src/copilot/copilotRemoteLauncher.ts +++ b/cli/src/copilot/copilotRemoteLauncher.ts @@ -264,6 +264,9 @@ export class CopilotRemoteLauncher extends RemoteLauncherBase { case 'error': this.messageBuffer.addMessage(message.message, 'status'); break; + case 'generated_image': + this.messageBuffer.addMessage(`Generated image: ${message.fileName}`, 'assistant'); + break; case 'turn_complete': this.messageBuffer.addMessage('Turn complete', 'status'); break; diff --git a/cli/src/cursor/cursorAcpRemoteLauncher.test.ts b/cli/src/cursor/cursorAcpRemoteLauncher.test.ts index a66867fa..e58021fc 100644 --- a/cli/src/cursor/cursorAcpRemoteLauncher.test.ts +++ b/cli/src/cursor/cursorAcpRemoteLauncher.test.ts @@ -17,7 +17,9 @@ const harness = vi.hoisted(() => ({ releaseSetConfigOption: null as (() => void) | null, deferLoadSession: null as Promise | null, releaseLoadSession: null as (() => void) | null, - stderrErrorHandler: null as ((error: { type: string; message: string; raw?: string }) => void) | null + stderrErrorHandler: null as ((error: { type: string; message: string; raw?: string }) => void) | null, + disconnectError: null as Error | null, + overlayCleanup: null as ReturnType | null })); const legacyLauncher = vi.hoisted(() => vi.fn()); @@ -114,7 +116,11 @@ vi.mock('./utils/cursorAcpBackend', () => ({ refreshSessionInfo: vi.fn(async () => {}), onPermissionRequest: vi.fn(), registerExtensionRequestHandler: vi.fn(), - disconnect: vi.fn(async () => {}) + disconnect: vi.fn(async () => { + if (harness.disconnectError) { + throw harness.disconnectError; + } + }) }; }) })); @@ -135,8 +141,18 @@ vi.mock('@/agent/permissionAdapter', () => ({ vi.mock('@/codex/utils/buildHapiMcpBridge', () => ({ buildHapiMcpBridge: async () => ({ server: { stop: () => {} }, - mcpServers: {} - }) + mcpServers: { + hapi: { command: 'hapi', args: ['mcp', '--url', 'http://127.0.0.1:1/'] }, + }, + }), +})); + +vi.mock('./utils/cursorMcpOverlay', () => ({ + cursorHapiMcpServerId: (sessionId: string) => `hapi-${sessionId}`, + installCursorMcpOverlay: () => { + harness.overlayCleanup = vi.fn(); + return { cleanup: harness.overlayCleanup }; + }, })); vi.mock('@/ui/ink/OpencodeDisplay', () => ({ @@ -178,8 +194,10 @@ function makeSession(sessionId: string | null): CursorSession { function makeClient() { return { + sessionId: 'test-session-id', rpcHandlerManager: { - registerHandler: vi.fn() + registerHandler: vi.fn(), + unregisterHandler: vi.fn() }, updateMetadata: vi.fn(), flushMetadata: vi.fn(async () => true), @@ -206,6 +224,8 @@ describe('cursorAcpRemoteLauncher', () => { harness.deferLoadSession = null; harness.releaseLoadSession = null; harness.stderrErrorHandler = null; + harness.disconnectError = null; + harness.overlayCleanup = null; legacyLauncher.mockClear(); process.stdin.isTTY = false; process.stdout.isTTY = false; @@ -224,6 +244,15 @@ describe('cursorAcpRemoteLauncher', () => { expect(legacyLauncher).not.toHaveBeenCalled(); }); + it('removes the Cursor MCP overlay even when backend.disconnect rejects', async () => { + harness.disconnectError = new Error('disconnect failed'); + const session = makeSession(null); + + await expect(cursorAcpRemoteLauncher(session)).rejects.toThrow('disconnect failed'); + expect(harness.overlayCleanup).toHaveBeenCalled(); + }); + + it('throws on initialize failure without invoking legacy launcher', async () => { harness.initializeError = new Error('agent acp not found'); const session = makeSession(null); diff --git a/cli/src/cursor/cursorAcpRemoteLauncher.ts b/cli/src/cursor/cursorAcpRemoteLauncher.ts index e0d35a9d..6b0ddaf8 100644 --- a/cli/src/cursor/cursorAcpRemoteLauncher.ts +++ b/cli/src/cursor/cursorAcpRemoteLauncher.ts @@ -34,10 +34,16 @@ import { readSharedCursorModelsCache } from '@/modules/common/cursorModelsShared import type { AcpSdkBackend } from '@/agent/backends/acp'; import type { AcpStderrError } from '@/agent/backends/acp/AcpStdioTransport'; import { registerAcpSessionTitleSync } from '@/agent/acpSessionTitle'; +import { + cursorHapiMcpServerId, + installCursorMcpOverlay, + type CursorMcpOverlayHandle, +} from './utils/cursorMcpOverlay'; import { resolveCursorSpawnModel, tryRemapCursorSpawnModelFromConnectError } from './utils/cursorStaleModelRemap'; + class CursorAcpRemoteLauncher extends RemoteLauncherBase { private readonly session: CursorSession; private backend: ReturnType | null = null; @@ -55,6 +61,8 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { private spawnedWithAutoReview = false; /** Avoid re-queueing `/auto-review` on every mid-session mode sync. */ private autoReviewSlashQueued = false; + private cursorMcpOverlay: CursorMcpOverlayHandle | null = null; + constructor(session: CursorSession) { super(process.env.DEBUG ? session.logPath : undefined); this.session = session; @@ -81,6 +89,24 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { }); this.happyServer = happyServer; + const hapiBridge = mcpServers.hapi; + if (hapiBridge) { + try { + this.cursorMcpOverlay = installCursorMcpOverlay(session.path, { + command: hapiBridge.command, + args: hapiBridge.args, + }, { + serverId: cursorHapiMcpServerId(session.client.sessionId), + }); + } catch (error) { + logger.warn( + '[cursor-acp] failed to install HAPI MCP overlay; continuing without inline media', + error, + ); + this.cursorMcpOverlay = { cleanup: () => {} }; + } + } + const autoReview = isCursorAutoReviewMode(session.getPermissionMode() as PermissionMode); this.spawnedWithAutoReview = autoReview; @@ -180,7 +206,8 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { ); const resumeSessionId = session.sessionId; - const mcpServerList = toAcpMcpServers(mcpServers); + // Cursor ACP ignores session/new|load mcpServers; native ~/.cursor/mcp.json is wired above. + const mcpServerList: McpServerStdio[] = []; let acpSessionId: string | undefined; for (let loadAttempt = 0; loadAttempt < 2; loadAttempt += 1) { @@ -249,7 +276,7 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { } else { acpSessionId = await backend.newSession({ cwd: session.path, - mcpServers: mcpServerList + mcpServers: mcpServerList, }); break; } @@ -380,31 +407,39 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { } protected async cleanup(): Promise { - this.clearAbortHandlers(this.session.client.rpcHandlerManager); - this.unregisterModelApplyHandler?.(); - this.unregisterModelApplyHandler = null; + // Capture overlay before awaited teardown so a reject from + // cancelAll/disconnect cannot leave a dead hapi-* entry in ~/.cursor/mcp.json. + const overlay = this.cursorMcpOverlay; + this.cursorMcpOverlay = null; - if (this.permissionAdapter) { - await this.permissionAdapter.cancelAll('Session ended'); - this.permissionAdapter = null; + try { + this.clearAbortHandlers(this.session.client.rpcHandlerManager); + this.unregisterModelApplyHandler?.(); + this.unregisterModelApplyHandler = null; + + if (this.permissionAdapter) { + await this.permissionAdapter.cancelAll('Session ended'); + this.permissionAdapter = null; + } + + if (this.extensionAdapter) { + await this.extensionAdapter.cancelAll('Session ended'); + this.extensionAdapter = null; + } + + if (this.backend) { + await this.backend.disconnect(); + this.backend = null; + } + + if (this.happyServer) { + this.happyServer.stop(); + this.happyServer = null; + } + } finally { + overlay?.cleanup(); + setCursorAcpModelsSnapshot(null); } - - if (this.extensionAdapter) { - await this.extensionAdapter.cancelAll('Session ended'); - this.extensionAdapter = null; - } - - if (this.backend) { - await this.backend.disconnect(); - this.backend = null; - } - - if (this.happyServer) { - this.happyServer.stop(); - this.happyServer = null; - } - - setCursorAcpModelsSnapshot(null); } private wireStderrErrorListener( @@ -483,6 +518,9 @@ class CursorAcpRemoteLauncher extends RemoteLauncherBase { case 'error': this.messageBuffer.addMessage(message.message, 'status'); break; + case 'generated_image': + this.messageBuffer.addMessage(`Generated image: ${message.fileName}`, 'assistant'); + break; case 'turn_complete': break; default: @@ -801,15 +839,6 @@ function syncCursorModelsFromAcp(backend: AcpSdkBackend, acpSessionId: string): seedCursorModelsCache(payload); } -function toAcpMcpServers(config: Record): McpServerStdio[] { - return Object.entries(config).map(([name, entry]) => ({ - name, - command: entry.command, - args: entry.args, - env: [] - })); -} - export async function cursorAcpRemoteLauncher(session: CursorSession): Promise<'switch' | 'exit'> { const launcher = new CursorAcpRemoteLauncher(session); return launcher.launch(); diff --git a/cli/src/cursor/utils/cursorAcpBackend.ts b/cli/src/cursor/utils/cursorAcpBackend.ts index f66304d0..43eeca50 100644 --- a/cli/src/cursor/utils/cursorAcpBackend.ts +++ b/cli/src/cursor/utils/cursorAcpBackend.ts @@ -81,7 +81,8 @@ export function createCursorAcpBackend(opts: CursorAcpBackendOptions): AcpSdkBac return new AcpSdkBackend({ command: 'agent', args: buildCursorAcpArgs(opts), - env: filterEnv(process.env) + env: filterEnv(process.env), + flavor: 'cursor', }); } diff --git a/cli/src/cursor/utils/cursorExtensionAdapter.test.ts b/cli/src/cursor/utils/cursorExtensionAdapter.test.ts index ed3d8d85..1d5aa4a6 100644 --- a/cli/src/cursor/utils/cursorExtensionAdapter.test.ts +++ b/cli/src/cursor/utils/cursorExtensionAdapter.test.ts @@ -1,8 +1,12 @@ import { describe, expect, it, vi, beforeEach } from 'vitest'; +import { mkdtempSync, writeFileSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; import type { ApiSessionClient } from '@/api/apiSession'; import type { AgentState } from '@/api/types'; import type { AgentMessage } from '@/agent/types'; import type { AcpSdkBackend } from '@/agent/backends/acp'; +import { clearGeneratedImages, getGeneratedImage } from '@/modules/common/generatedImages'; import { CursorExtensionAdapter } from './cursorExtensionAdapter'; type ExtensionHandler = (params: unknown, requestId: string | number | null) => Promise; @@ -275,6 +279,125 @@ describe('CursorExtensionAdapter', () => { ]); }); + it('registers cursor/generate_image base64 imageData and emits generated_image', async () => { + clearGeneratedImages(); + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + const { handlers, getMessages } = createHarness(); + await handlers.get('cursor/generate_image')!({ + toolCallId: 'img-1', + description: 'App icon', + filePath: '/tmp/icon.png', + imageData: pngHeader.toString('base64'), + }, null); + + const messages = getMessages(); + expect(messages[0]).toMatchObject({ + type: 'tool_call', + id: 'img-1', + name: 'CursorGenerateImage', + status: 'completed', + }); + expect(messages[1]).toMatchObject({ + type: 'generated_image', + fileName: 'icon.png', + mimeType: 'image/png', + source: { + ingress: 'acp', + flavor: 'cursor', + toolCallId: 'img-1', + toolName: 'cursor/generate_image', + }, + }); + expect(messages[2]).toMatchObject({ + type: 'tool_result', + id: 'img-1', + status: 'completed', + }); + + const generated = messages[1]; + expect(generated.type).toBe('generated_image'); + if (generated.type === 'generated_image') { + expect(getGeneratedImage(generated.imageId)?.mimeType).toBe('image/png'); + } + clearGeneratedImages(); + }); + + it('does not read filePath-only generate_image (permission bypass)', async () => { + const dir = mkdtempSync(join(tmpdir(), 'hapi-cursor-gen-img-')); + try { + const filePath = join(dir, 'secret.png'); + writeFileSync(filePath, Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a])); + + const { handlers, getMessages } = createHarness(); + await handlers.get('cursor/generate_image')!({ + toolCallId: 'img-path', + description: 'Must not auto-read disk', + filePath, + }, null); + + expect(getMessages().map((m) => m.type)).toEqual(['tool_call', 'tool_result']); + expect(getMessages().some((m) => m.type === 'generated_image')).toBe(false); + } finally { + rmSync(dir, { recursive: true, force: true }); + clearGeneratedImages(); + } + }); + + it('registers cursor/generate_image base64 imageData when filePath is absent', async () => { + clearGeneratedImages(); + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]); + const { handlers, getMessages } = createHarness(); + await handlers.get('cursor/generate_image')!({ + toolCallId: 'img-2', + description: 'Inline bytes', + imageData: pngHeader.toString('base64'), + }, null); + + const generated = getMessages().find((m) => m.type === 'generated_image'); + expect(generated).toMatchObject({ + type: 'generated_image', + mimeType: 'image/png', + source: { + ingress: 'acp', + flavor: 'cursor', + toolCallId: 'img-2', + toolName: 'cursor/generate_image', + }, + }); + clearGeneratedImages(); + }); + + it('rejects oversized generate_image base64 before decode', async () => { + const { handlers, getMessages } = createHarness(); + const huge = 'A'.repeat(Math.ceil(25 * 1024 * 1024 * 4 / 3) + 5); + await handlers.get('cursor/generate_image')!({ + toolCallId: 'img-huge', + description: 'Too big', + imageData: huge, + }, null); + + expect(getMessages().map((m) => m.type)).toEqual(['tool_call', 'tool_result']); + expect(getMessages().some((m) => m.type === 'generated_image')).toBe(false); + const toolCall = getMessages()[0]; + expect(toolCall).toMatchObject({ + type: 'tool_call', + input: expect.objectContaining({ imageDataChars: huge.length }), + }); + if (toolCall.type === 'tool_call') { + expect(toolCall.input).not.toHaveProperty('imageData'); + } + }); + + it('still emits tool_call/result when generate_image has no path or bytes', async () => { + const { handlers, getMessages } = createHarness(); + await handlers.get('cursor/generate_image')!({ + toolCallId: 'img-3', + description: 'No media yet', + }, null); + + expect(getMessages().map((m) => m.type)).toEqual(['tool_call', 'tool_result']); + }); + it('cancelAll resolves pending extension requests as cancelled', async () => { const { handlers, adapter, getAgentState } = createHarness(); const askPending = handlers.get('cursor/ask_question')!({ toolCallId: 'q-cancel' }, null); diff --git a/cli/src/cursor/utils/cursorExtensionAdapter.ts b/cli/src/cursor/utils/cursorExtensionAdapter.ts index f452a4d3..ab28997c 100644 --- a/cli/src/cursor/utils/cursorExtensionAdapter.ts +++ b/cli/src/cursor/utils/cursorExtensionAdapter.ts @@ -5,6 +5,13 @@ import { logger } from '@/ui/logger'; import { asString, isObject } from '@hapi/protocol'; import type { AgentMessage, PlanItem } from '@/agent/types'; import { randomUUID } from 'node:crypto'; +import { basename } from 'node:path'; +import { + decodeGeneratedImageBase64, + detectImageMimeType, + registerGeneratedImage, +} from '@/modules/common/generatedImages'; +import type { InlineMediaSource } from '@/modules/common/inlineMediaSource'; type PendingExtensionRequest = { tool: string; @@ -64,7 +71,7 @@ export class CursorExtensionAdapter { }); this.backend.registerExtensionRequestHandler('cursor/generate_image', async (params) => { - this.handleGenerateImage(params); + await this.handleGenerateImage(params); return {}; }); } @@ -196,20 +203,47 @@ export class CursorExtensionAdapter { } } - private handleGenerateImage(params: unknown): void { + private async handleGenerateImage(params: unknown): Promise { if (!isObject(params)) return; const toolCallId = extractToolCallId(params) ?? `cursor-image-${randomUUID()}`; + const safeParams = summarizeGenerateImageParams(params); this.onMessage({ type: 'tool_call', id: toolCallId, name: 'CursorGenerateImage', - input: params, + input: safeParams, status: 'completed' }); + + const image = await registerCursorGeneratedImage(params); + if (image) { + const source: InlineMediaSource = { + ingress: 'acp', + flavor: 'cursor', + toolCallId, + toolName: 'cursor/generate_image', + }; + this.onMessage({ + type: 'generated_image', + imageId: image.id, + fileName: image.fileName, + mimeType: image.mimeType, + source, + }); + } else { + const imageData = asString(params.imageData) + ?? asString(params.image_data) + ?? asString(params.data); + logger.debug('[cursor-acp] cursor/generate_image rejected', { + toolCallId, + imageDataChars: typeof imageData === 'string' ? imageData.length : 0, + }); + } + this.onMessage({ type: 'tool_result', id: toolCallId, - output: params, + output: safeParams, status: 'completed' }); } @@ -258,6 +292,69 @@ function wrapOutcome(outcome: T): { outcome: T } return { outcome }; } +async function registerCursorGeneratedImage(params: Record) { + const filePath = asString(params.filePath) + ?? asString(params.file_path) + ?? asString(params.path) + ?? asString(params.imagePath) + ?? asString(params.image_path); + + const imageData = asString(params.imageData) + ?? asString(params.image_data) + ?? asString(params.data); + + // Only inline bytes are safe here. Path-only reads would bypass the + // permission-gated display_image / display_video / display_media MCP tools (same class as + // URI-only ACP image blocks). Path support needs an explicit approval flow. + if (imageData) { + try { + const bytes = decodeGeneratedImageBase64(imageData); + if (!bytes) { + return null; + } + const mimeType = detectImageMimeType(bytes); + if (!mimeType) { + return null; + } + const path = filePath ?? `${randomUUID()}.bin`; + return registerGeneratedImage({ + id: randomUUID(), + path, + fileName: basename(path), + mimeType, + bytes, + }); + } catch (error) { + logger.debug('[cursor-acp] failed to register generate_image base64 payload', error); + return null; + } + } + + if (filePath) { + logger.debug( + '[cursor-acp] ignoring cursor/generate_image filePath without inline bytes; use display_image/display_video MCP for local paths', + { filePath }, + ); + } + + return null; +} + +/** Drop raw base64 from chat/logs; keep length so rejects stay diagnosable. */ +function summarizeGenerateImageParams(params: Record): Record { + const imageData = asString(params.imageData) + ?? asString(params.image_data) + ?? asString(params.data); + const summary: Record = { ...params }; + delete summary.imageData; + delete summary.image_data; + delete summary.data; + if (typeof imageData === 'string') { + summary.imageDataChars = imageData.length; + } + return summary; +} + function extractToolCallId(params: unknown): string | null { if (!isObject(params)) return null; return asString(params.toolCallId); diff --git a/cli/src/cursor/utils/cursorMcpOverlay.test.ts b/cli/src/cursor/utils/cursorMcpOverlay.test.ts new file mode 100644 index 00000000..d5a2f182 --- /dev/null +++ b/cli/src/cursor/utils/cursorMcpOverlay.test.ts @@ -0,0 +1,546 @@ +import { afterEach, describe, expect, it } from 'vitest'; +import { spawnSync } from 'node:child_process'; +import { + existsSync, + linkSync, + lstatSync, + mkdirSync, + readFileSync, + readdirSync, + rmSync, + statSync, + symlinkSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { join } from 'node:path'; +import { homedir, tmpdir } from 'node:os'; +import { randomUUID } from 'node:crypto'; +import { + CURSOR_HAPI_MCP_SERVER_ID, + HAPI_MCP_OVERLAY_PID_ENV, + cursorHapiMcpServerId, + installCursorMcpOverlay, + isProcessAlive, + readLockOwner, + resolveCursorMcpConfigDir, + withMcpJsonLock, + writeMcpJsonAtomic, +} from './cursorMcpOverlay'; + +describe('installCursorMcpOverlay', () => { + const roots: string[] = []; + /** Unit tests must not shell out to a real Cursor `agent` binary. */ + const noopEnable = () => ({ status: 0 }); + + afterEach(() => { + for (const root of roots.splice(0)) { + rmSync(root, { recursive: true, force: true }); + } + }); + + function makeProjectDir(initialMcpJson?: string): string { + const root = join(tmpdir(), `hapi-cursor-mcp-${randomUUID()}`); + mkdirSync(root, { recursive: true }); + roots.push(root); + if (initialMcpJson !== undefined) { + mkdirSync(join(root, '.cursor'), { recursive: true }); + writeFileSync(join(root, '.cursor', 'mcp.json'), initialMcpJson, 'utf-8'); + } + return root; + } + + it('defaults MCP config dir to ~/.cursor (outside the project tree)', () => { + expect(resolveCursorMcpConfigDir()).toBe(join(homedir(), '.cursor')); + expect(resolveCursorMcpConfigDir(' /tmp/custom-cursor ')).toBe('/tmp/custom-cursor'); + }); + + it('writes per-session bridge into .cursor/mcp.json and removes only that id on cleanup', () => { + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-a'); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + const merged = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(merged.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + expect(merged.mcpServers[serverId]).toEqual({ + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + env: { [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid) }, + }); + expect(merged.mcpServers[CURSOR_HAPI_MCP_SERVER_ID]).toBeUndefined(); + + handle.cleanup(); + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + expect(after.mcpServers[serverId]).toBeUndefined(); + }); + + it('leaves a newer session bridge intact when an older session cleans up first', () => { + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const idA = cursorHapiMcpServerId('session-a'); + const idB = cursorHapiMcpServerId('session-b'); + + const handleA = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:1111/'], + }, { serverId: idA, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + const handleB = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:2222/'], + }, { serverId: idB, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + handleA.cleanup(); + + const afterA = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(afterA.mcpServers[idA]).toBeUndefined(); + expect(afterA.mcpServers[idB]).toEqual({ + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:2222/'], + env: { [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid) }, + }); + + handleB.cleanup(); + + const afterB = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(afterB.mcpServers[idB]).toBeUndefined(); + expect(afterB.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + }); + + it('preserves mcpServers keys added during the session on cleanup', () => { + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-a'); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + writeFileSync(mcpPath, JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + [serverId]: { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, + concurrent: { command: 'npx', args: ['-y', 'some-mcp'] }, + }, + }, null, 2) + '\n', 'utf-8'); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + expect(after.mcpServers.concurrent).toEqual({ command: 'npx', args: ['-y', 'some-mcp'] }); + expect(after.mcpServers[serverId]).toBeUndefined(); + }); + + it('preserves env-only concurrent edits on the overlay entry during cleanup', () => { + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-env'); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + writeFileSync(mcpPath, JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + [serverId]: { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + env: { + [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid), + USER_TOKEN: 'keep-me', + }, + }, + }, + }, null, 2) + '\n', 'utf-8'); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record }>; + }; + expect(after.mcpServers[serverId]).toEqual({ + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + env: { + [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid), + USER_TOKEN: 'keep-me', + }, + }); + }); + + it('restores a pre-existing entry for the same server id instead of deleting it', () => { + const serverId = cursorHapiMcpServerId('session-a'); + const prior = { command: 'old-hapi', args: ['mcp'] }; + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + [serverId]: prior, + }, + }, null, 2)); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers[serverId]).toEqual(prior); + }); + + it('does not touch a legacy shared hapi key when using a per-session id', () => { + const legacyHapi = { command: 'user-hapi', args: ['mcp', '--custom'] }; + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + [CURSOR_HAPI_MCP_SERVER_ID]: legacyHapi, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-a'); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers[CURSOR_HAPI_MCP_SERVER_ID]).toEqual(legacyHapi); + expect(after.mcpServers[serverId]).toBeUndefined(); + }); + + it('preserves a mid-session replacement of the session entry on cleanup', () => { + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-a'); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + const userOwned = { command: 'user-hapi', args: ['mcp', '--custom'] }; + writeFileSync(mcpPath, JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + [serverId]: userOwned, + }, + }, null, 2) + '\n', 'utf-8'); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers[serverId]).toEqual(userOwned); + expect(after.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + }); + + it('creates .cursor/mcp.json when missing and removes file when only the session entry was present', () => { + const cwd = makeProjectDir(); + const serverId = cursorHapiMcpServerId('session-a'); + expect(existsSync(join(cwd, '.cursor', 'mcp.json'))).toBe(false); + + const handle = installCursorMcpOverlay(cwd, { + command: 'hapi', + args: ['mcp', '--url', 'http://127.0.0.1:9999/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + expect(existsSync(mcpPath)).toBe(true); + + handle.cleanup(); + expect(existsSync(mcpPath)).toBe(false); + }); + + it('throws when existing .cursor/mcp.json is not valid JSON', () => { + const cwd = makeProjectDir('{ not-json'); + expect(() => installCursorMcpOverlay(cwd, { + command: 'hapi', + args: ['mcp', '--url', 'http://127.0.0.1:9999/'], + }, { serverId: cursorHapiMcpServerId('session-a'), enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') })).toThrow(); + // Malformed project config must stay untouched for the launcher try/catch path. + expect(readFileSync(join(cwd, '.cursor', 'mcp.json'), 'utf-8')).toBe('{ not-json'); + }); + + it('prunes dead hapi-* overlays stamped with HAPI_MCP_OVERLAY_PID on install', () => { + // spawnSync waits for exit; the returned pid is then dead. + const probe = spawnSync(process.execPath, ['-e', ''], { encoding: 'utf-8' }); + const exitedPid = probe.pid; + expect(typeof exitedPid).toBe('number'); + expect(isProcessAlive(exitedPid!)).toBe(false); + + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + 'hapi-dead': { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:1111/'], + env: { [HAPI_MCP_OVERLAY_PID_ENV]: String(exitedPid) }, + }, + 'hapi-live': { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:2222/'], + env: { [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid) }, + }, + 'hapi-user': { + command: 'user-owned', + args: [], + }, + }, + }, null, 2)); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + const serverId = cursorHapiMcpServerId('session-a'); + + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:3333/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + const merged = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record }>; + }; + expect(merged.mcpServers['hapi-dead']).toBeUndefined(); + expect(merged.mcpServers['hapi-live']?.command).toBe('/bin/hapi'); + expect(merged.mcpServers['hapi-user']?.command).toBe('user-owned'); + expect(merged.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + expect(merged.mcpServers[serverId]?.env?.[HAPI_MCP_OVERLAY_PID_ENV]).toBe(String(process.pid)); + + handle.cleanup(); + }); + + it('refuses a symlinked .cursor/mcp.json and leaves the external target unchanged', () => { + const cwd = makeProjectDir(); + const cursorDir = join(cwd, '.cursor'); + mkdirSync(cursorDir, { recursive: true }); + const realConfig = join(cwd, 'shared-mcp.json'); + const original = `${JSON.stringify({ + mcpServers: { + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)}\n`; + writeFileSync(realConfig, original, 'utf-8'); + const mcpPath = join(cursorDir, 'mcp.json'); + symlinkSync(realConfig, mcpPath); + + expect(() => installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId: cursorHapiMcpServerId('session-a'), enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') })).toThrow( + /Refusing to write a symlinked Cursor MCP config/ + ); + + expect(lstatSync(mcpPath).isSymbolicLink()).toBe(true); + expect(readFileSync(realConfig, 'utf-8')).toBe(original); + }); + + it('refuses a symlinked .cursor directory before mutating MCP config', () => { + const cwd = makeProjectDir(); + const realCursorDir = join(cwd, 'real-cursor'); + mkdirSync(realCursorDir, { recursive: true }); + const externalMcp = join(realCursorDir, 'mcp.json'); + const original = `${JSON.stringify({ mcpServers: {} }, null, 2)}\n`; + writeFileSync(externalMcp, original, 'utf-8'); + symlinkSync(realCursorDir, join(cwd, '.cursor')); + + expect(() => installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId: cursorHapiMcpServerId('session-a'), enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') })).toThrow( + /Refusing to use a symlinked Cursor config directory/ + ); + + expect(readFileSync(externalMcp, 'utf-8')).toBe(original); + }); + + it('writeMcpJsonAtomic preserves restrictive mode and cleans up tmp on failure path', () => { + const cwd = makeProjectDir(); + mkdirSync(join(cwd, '.cursor'), { recursive: true }); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + writeFileSync(mcpPath, `${JSON.stringify({ mcpServers: {} }, null, 2)}\n`, { + encoding: 'utf-8', + mode: 0o600, + }); + + writeMcpJsonAtomic(mcpPath, { + mcpServers: { a: { command: 'a', args: [] } }, + }); + + expect(statSync(mcpPath).mode & 0o777).toBe(0o600); + expect(JSON.parse(readFileSync(mcpPath, 'utf-8')).mcpServers.a.command).toBe('a'); + expect(readdirSync(join(cwd, '.cursor')).filter((name) => name.endsWith('.tmp'))).toEqual([]); + }); + + it('writeMcpJsonAtomic replaces via rename and withMcpJsonLock serializes writers', () => { + const cwd = makeProjectDir(); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + mkdirSync(join(cwd, '.cursor'), { recursive: true }); + const lockPath = `${mcpPath}.hapi.lock`; + + writeMcpJsonAtomic(mcpPath, { + mcpServers: { a: { command: 'a', args: [] } }, + }); + expect(JSON.parse(readFileSync(mcpPath, 'utf-8')).mcpServers.a.command).toBe('a'); + + const order: string[] = []; + withMcpJsonLock(lockPath, () => { + order.push('outer-enter'); + expect(readLockOwner(lockPath)?.pid).toBe(process.pid); + // Second exclusive link onto the same path must fail while held. + const other = `${lockPath}.other.tmp`; + writeFileSync(other, JSON.stringify({ pid: process.pid, token: 'other' }), { + encoding: 'utf-8', + mode: 0o600, + }); + expect(() => linkSync(other, lockPath)).toThrow(); + unlinkSync(other); + order.push('outer-exit'); + }); + expect(order).toEqual(['outer-enter', 'outer-exit']); + expect(existsSync(lockPath)).toBe(false); + }); + + it('withMcpJsonLock only unlinks its own token (does not delete a successor lock)', () => { + const cwd = makeProjectDir(); + mkdirSync(join(cwd, '.cursor'), { recursive: true }); + const lockPath = join(cwd, '.cursor', 'mcp.json.hapi.lock'); + + let releasedOwnerToken: string | undefined; + withMcpJsonLock(lockPath, () => { + const owner = readLockOwner(lockPath); + expect(owner?.pid).toBe(process.pid); + releasedOwnerToken = owner?.token; + // Simulate a successor stealing the path while we still hold the fd conceptually: + // write a different owner into the lock path after our create (race successor). + writeFileSync(lockPath, JSON.stringify({ pid: process.pid, token: 'successor-token' }), 'utf-8'); + }); + // Original owner must not unlink the successor's lock. + expect(existsSync(lockPath)).toBe(true); + expect(readLockOwner(lockPath)?.token).toBe('successor-token'); + expect(releasedOwnerToken).toBeTruthy(); + unlinkSync(lockPath); + }); + + it('cleanup preserves concurrent top-level mcp.json fields when servers are empty', () => { + const cwd = makeProjectDir(); + const serverId = cursorHapiMcpServerId('session-a'); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + + const handle = installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { serverId, enableCursorMcp: noopEnable, mcpConfigDir: join(cwd, '.cursor') }); + + writeFileSync(mcpPath, JSON.stringify({ + mcpServers: { + [serverId]: { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, + }, + inputs: [{ id: 'keep-me' }], + }, null, 2) + '\n', 'utf-8'); + + handle.cleanup(); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + inputs: unknown[]; + }; + expect(after.mcpServers[serverId]).toBeUndefined(); + expect(after.inputs).toEqual([{ id: 'keep-me' }]); + expect(existsSync(mcpPath)).toBe(true); + }); + + it('isProcessAlive treats EPERM as alive and ESRCH as dead', () => { + expect(isProcessAlive(process.pid)).toBe(true); + expect(isProcessAlive(2_147_483_646)).toBe(false); + }); + + it('fails closed on a stale lock instead of pathname-stealing', () => { + const cwd = makeProjectDir(); + mkdirSync(join(cwd, '.cursor'), { recursive: true }); + const lockPath = join(cwd, '.cursor', 'mcp.json.hapi.lock'); + writeFileSync(lockPath, JSON.stringify({ pid: 2_147_483_646, token: 'dead-owner' }), 'utf-8'); + + expect(() => withMcpJsonLock(lockPath, () => {})).toThrow(/Stale Cursor MCP overlay lock/); + expect(readLockOwner(lockPath)?.token).toBe('dead-owner'); + }); + + it('rolls back mcp.json and throws when agent mcp enable fails', () => { + const prior = { command: 'user-hapi', args: ['mcp'] }; + const cwd = makeProjectDir(JSON.stringify({ + mcpServers: { + [CURSOR_HAPI_MCP_SERVER_ID]: prior, + other: { command: 'echo', args: ['x'] }, + }, + }, null, 2)); + const serverId = cursorHapiMcpServerId('session-a'); + const mcpPath = join(cwd, '.cursor', 'mcp.json'); + + expect(() => installCursorMcpOverlay(cwd, { + command: '/bin/hapi', + args: ['mcp', '--url', 'http://127.0.0.1:12345/'], + }, { + serverId, + enableCursorMcp: () => ({ status: 1, stderr: 'enable denied' }), + mcpConfigDir: join(cwd, '.cursor'), + })).toThrow(/agent mcp enable/); + + const after = JSON.parse(readFileSync(mcpPath, 'utf-8')) as { + mcpServers: Record; + }; + expect(after.mcpServers[serverId]).toBeUndefined(); + expect(after.mcpServers[CURSOR_HAPI_MCP_SERVER_ID]).toEqual(prior); + expect(after.mcpServers.other).toEqual({ command: 'echo', args: ['x'] }); + }); +}); diff --git a/cli/src/cursor/utils/cursorMcpOverlay.ts b/cli/src/cursor/utils/cursorMcpOverlay.ts new file mode 100644 index 00000000..7358fa09 --- /dev/null +++ b/cli/src/cursor/utils/cursorMcpOverlay.ts @@ -0,0 +1,396 @@ +/** + * Cursor ACP does not connect MCP servers passed on session/new (upstream limitation). + * The working path is user-level `~/.cursor/mcp.json` + `agent mcp enable ` (spawned + * with the session cwd). Project `.cursor/mcp.json` is intentionally avoided so ephemeral + * `hapi-` bridges cannot be `git add`ed from the checked-out tree. + * See https://forum.cursor.com/t/acp-agent-silently-ignores-mcpservers-in-session-new/153623 + */ + +import { + existsSync, + linkSync, + lstatSync, + mkdirSync, + readFileSync, + renameSync, + rmSync, + statSync, + unlinkSync, + writeFileSync, +} from 'node:fs'; +import { homedir } from 'node:os'; +import { join } from 'node:path'; +import { randomUUID } from 'node:crypto'; +import { spawnSync } from 'node:child_process'; +import { logger } from '@/ui/logger'; + +/** Historical fixed id — prefer {@link cursorHapiMcpServerId} so concurrent sessions do not share one key. */ +export const CURSOR_HAPI_MCP_SERVER_ID = 'hapi'; + +/** + * Per-session MCP server id for user-level `~/.cursor/mcp.json`. + * Concurrent sessions must not share a single `hapi` key — cleanup of an + * older session would otherwise restore a dead loopback URL over a newer live bridge. + */ +export function cursorHapiMcpServerId(sessionId: string): string { + const trimmed = sessionId.trim(); + if (!trimmed) { + throw new Error('sessionId is required for Cursor HAPI MCP overlay'); + } + return `hapi-${trimmed}`; +} + +/** Resolve the Cursor MCP config directory (override for tests; default `~/.cursor`). */ +export function resolveCursorMcpConfigDir(override?: string): string { + const trimmed = override?.trim(); + return trimmed && trimmed.length > 0 ? trimmed : join(homedir(), '.cursor'); +} + +type McpServerEntry = { + command: string; + args: string[]; + env?: Record; +}; + +/** Marks HAPI-owned overlay entries so a later launch can prune dead PIDs. */ +export const HAPI_MCP_OVERLAY_PID_ENV = 'HAPI_MCP_OVERLAY_PID'; + +type CursorMcpJson = { + mcpServers?: Record; +}; + +type LockOwner = { + pid: number; + token: string; +}; + +export type CursorMcpOverlayHandle = { + cleanup: () => void; +}; + +type EnableCursorMcpResult = { + status: number | null; + stdout?: string | null; + stderr?: string | null; +}; + +export type EnableCursorMcp = (cwd: string, id: string) => EnableCursorMcpResult; + +const LOCK_RETRY_INTERVAL_MS = 50; +const MAX_LOCK_ATTEMPTS = 100; + +function defaultEnableCursorMcp(cwd: string, id: string): EnableCursorMcpResult { + return spawnSync('agent', ['mcp', 'enable', id], { + cwd, + encoding: 'utf-8', + timeout: 30_000, + }); +} + +function parseMcpJson(raw: string): CursorMcpJson { + const parsed = JSON.parse(raw) as unknown; + if (parsed === null || typeof parsed !== 'object') { + return { mcpServers: {} }; + } + return parsed as CursorMcpJson; +} + +function readMcpJson(path: string): CursorMcpJson { + if (!existsSync(path)) { + return { mcpServers: {} }; + } + return parseMcpJson(readFileSync(path, 'utf-8')); +} + +/** + * Atomic replace so readers never see a partial mcp.json; preserves existing mode. + * Refuses to write through a symlink — a project-controlled link could point + * outside `cwd`, and cleanup is not a byte-for-byte restore of the target. + */ +export function writeMcpJsonAtomic(path: string, config: CursorMcpJson): void { + const entry = lstatSync(path, { throwIfNoEntry: false }); + if (entry?.isSymbolicLink()) { + throw new Error(`Refusing to write a symlinked Cursor MCP config: ${path}`); + } + const mode = existsSync(path) ? (statSync(path).mode & 0o777) : 0o600; + const tmp = `${path}.${process.pid}.${randomUUID()}.tmp`; + try { + writeFileSync(tmp, `${JSON.stringify(config, null, 2)}\n`, { + encoding: 'utf-8', + mode, + }); + renameSync(tmp, path); + } finally { + rmSync(tmp, { force: true }); + } +} + +function sleepSync(ms: number): void { + Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, ms); +} + +export function readLockOwner(lockPath: string): LockOwner | null { + try { + const parsed = JSON.parse(readFileSync(lockPath, 'utf-8')) as unknown; + if ( + parsed !== null + && typeof parsed === 'object' + && typeof (parsed as LockOwner).pid === 'number' + && typeof (parsed as LockOwner).token === 'string' + ) { + return parsed as LockOwner; + } + } catch { + // corrupt / empty lock + } + return null; +} + +/** Fail closed: only ESRCH means the PID is confirmed gone. EPERM ⇒ alive. */ +export function isProcessAlive(pid: number): boolean { + if (pid <= 0) { + return false; + } + try { + process.kill(pid, 0); + return true; + } catch (error) { + const code = error && typeof error === 'object' && 'code' in error + ? (error as { code?: string }).code + : undefined; + return code !== 'ESRCH'; + } +} + +/** + * Exclusive cross-process lock for mcp.json read-modify-write. + * Owner JSON is published atomically via link(2) from a fully-written staging file. + * Stale/dead locks fail closed — pathname check-then-unlink/rename can steal a + * successor's live lock under concurrent recoverers. Release unlinks only when + * the path still holds this owner's token. + */ +export function withMcpJsonLock(lockPath: string, fn: () => void): void { + let attempts = 0; + let owner: LockOwner | undefined; + + while (attempts < MAX_LOCK_ATTEMPTS) { + owner = { pid: process.pid, token: randomUUID() }; + const candidate = `${lockPath}.${owner.token}.tmp`; + try { + writeFileSync(candidate, JSON.stringify(owner), { + encoding: 'utf-8', + flag: 'wx', + mode: 0o600, + }); + try { + linkSync(candidate, lockPath); + break; + } catch (err: unknown) { + const code = err && typeof err === 'object' && 'code' in err + ? (err as { code?: string }).code + : undefined; + if (code !== 'EEXIST') { + throw err; + } + attempts++; + const existing = readLockOwner(lockPath); + if (existing && isProcessAlive(existing.pid)) { + sleepSync(LOCK_RETRY_INTERVAL_MS); + } else { + throw new Error( + `Stale Cursor MCP overlay lock: ${lockPath}; remove it and retry ` + + `(e.g. rm -f ${JSON.stringify(lockPath)})` + ); + } + } finally { + rmSync(candidate, { force: true }); + } + continue; + } catch (err: unknown) { + rmSync(candidate, { force: true }); + if (err instanceof Error && err.message.startsWith('Stale Cursor MCP overlay lock:')) { + throw err; + } + const code = err && typeof err === 'object' && 'code' in err + ? (err as { code?: string }).code + : undefined; + if (code === 'EEXIST') { + attempts++; + sleepSync(LOCK_RETRY_INTERVAL_MS); + continue; + } + throw err; + } + } + + if (!owner || !existsSync(lockPath) || readLockOwner(lockPath)?.token !== owner.token) { + throw new Error(`Timed out waiting for Cursor MCP overlay lock: ${lockPath}`); + } + + try { + fn(); + } finally { + try { + if (readLockOwner(lockPath)?.token === owner.token) { + unlinkSync(lockPath); + } + } catch { + // ignore + } + } +} + +function comparableMcpEnv(env?: Record): string { + // Ignore the HAPI PID stamp (rewritten on install / crash-recovery), but + // treat any other env edit as a concurrent user change that must survive cleanup. + return JSON.stringify( + Object.entries(env ?? {}) + .filter(([key]) => key !== HAPI_MCP_OVERLAY_PID_ENV) + .sort(([left], [right]) => left.localeCompare(right)), + ); +} + +function sameMcpEntry(a: McpServerEntry | undefined, b: McpServerEntry | undefined): boolean { + if (!a || !b) { + return a === b; + } + return a.command === b.command + && JSON.stringify(a.args) === JSON.stringify(b.args) + && comparableMcpEnv(a.env) === comparableMcpEnv(b.env); +} + +/** + * Merge the per-session HAPI stdio bridge into `~/.cursor/mcp.json` (or + * `options.mcpConfigDir`) and approve it for Cursor's native MCP loader. + * + * Cleanup undoes only the exact entry this session installed under `serverId` (or restores a + * pre-existing value for that same id). Concurrent edits to other mcpServers keys — and to + * this id when it no longer matches the installed overlay — survive the session. + * + * Install and cleanup serialize via a lockfile and write mcp.json atomically so concurrent + * CLI processes cannot clobber each other's `hapi-*` entries. + */ +export function installCursorMcpOverlay( + cwd: string, + bridge: { command: string; args: string[] }, + options: { + serverId: string; + enableCursorMcp?: EnableCursorMcp; + /** Override config dir (tests). Production uses `~/.cursor`. */ + mcpConfigDir?: string; + }, +): CursorMcpOverlayHandle { + const serverId = options.serverId.trim(); + if (!serverId) { + throw new Error('serverId is required for Cursor HAPI MCP overlay'); + } + + const cursorDir = resolveCursorMcpConfigDir(options.mcpConfigDir); + const mcpJsonPath = join(cursorDir, 'mcp.json'); + const lockPath = `${mcpJsonPath}.hapi.lock`; + const cursorDirEntry = lstatSync(cursorDir, { throwIfNoEntry: false }); + if (cursorDirEntry?.isSymbolicLink()) { + throw new Error(`Refusing to use a symlinked Cursor config directory: ${cursorDir}`); + } + mkdirSync(cursorDir, { recursive: true }); + + const installedHapi: McpServerEntry = { + command: bridge.command, + args: [...bridge.args], + env: { [HAPI_MCP_OVERLAY_PID_ENV]: String(process.pid) }, + }; + + let hadFile = false; + let hadServer = false; + let previousServer: McpServerEntry | undefined; + + withMcpJsonLock(lockPath, () => { + hadFile = existsSync(mcpJsonPath); + const previous = hadFile ? readMcpJson(mcpJsonPath) : { mcpServers: {} as Record }; + previous.mcpServers ??= {}; + + // Crash recovery: drop prior hapi-* overlays whose owner PID is gone. + // Only prune entries we stamped with HAPI_MCP_OVERLAY_PID — never user-owned keys. + for (const [id, entry] of Object.entries(previous.mcpServers)) { + if (!id.startsWith('hapi-')) { + continue; + } + const pidRaw = entry.env?.[HAPI_MCP_OVERLAY_PID_ENV]; + if (typeof pidRaw !== 'string' || pidRaw.trim() === '') { + continue; + } + const pid = Number(pidRaw); + if (!Number.isSafeInteger(pid) || pid <= 0) { + continue; + } + if (!isProcessAlive(pid)) { + delete previous.mcpServers[id]; + } + } + + hadServer = Object.prototype.hasOwnProperty.call(previous.mcpServers, serverId); + previousServer = hadServer ? previous.mcpServers[serverId] : undefined; + + const config: CursorMcpJson = { + ...previous, + mcpServers: { + ...previous.mcpServers, + [serverId]: installedHapi, + }, + }; + writeMcpJsonAtomic(mcpJsonPath, config); + }); + + const cleanup = (): void => { + try { + withMcpJsonLock(lockPath, () => { + if (!existsSync(mcpJsonPath)) { + return; + } + + const current = readMcpJson(mcpJsonPath); + current.mcpServers ??= {}; + + const currentServer = current.mcpServers[serverId]; + if (!sameMcpEntry(currentServer, installedHapi)) { + // User/Cursor replaced or removed our overlay entry — leave alone. + return; + } + + if (hadServer && previousServer) { + current.mcpServers[serverId] = previousServer; + } else { + delete current.mcpServers[serverId]; + } + + const { mcpServers, ...otherTopLevel } = current; + const remainingServers = Object.keys(mcpServers ?? {}); + if ( + !hadFile + && remainingServers.length === 0 + && Object.keys(otherTopLevel).length === 0 + ) { + rmSync(mcpJsonPath, { force: true }); + return; + } + + writeMcpJsonAtomic(mcpJsonPath, current); + }); + } catch (error) { + logger.debug('[cursor-acp] cursor MCP overlay cleanup failed', error); + } + }; + + const enable = (options.enableCursorMcp ?? defaultEnableCursorMcp)(cwd, serverId); + + if (enable.status !== 0) { + const detail = (enable.stderr || enable.stdout || '').trim(); + cleanup(); + throw new Error( + `agent mcp enable ${serverId} failed (status=${enable.status ?? 'null'}${detail ? `: ${detail}` : ''})` + ); + } + + logger.debug(`[cursor-acp] enabled native MCP server ${serverId} via ${mcpJsonPath}`); + return { cleanup }; +} diff --git a/cli/src/grok/grokRemoteLauncher.ts b/cli/src/grok/grokRemoteLauncher.ts index e87dd548..fe591848 100644 --- a/cli/src/grok/grokRemoteLauncher.ts +++ b/cli/src/grok/grokRemoteLauncher.ts @@ -413,6 +413,9 @@ class GrokRemoteLauncher extends RemoteLauncherBase { case 'error': this.messageBuffer.addMessage(message.message, 'status') break + case 'generated_image': + this.messageBuffer.addMessage(`Generated image: ${message.fileName}`, 'assistant') + break case 'turn_complete': this.messageBuffer.addMessage('Turn complete', 'status') break diff --git a/cli/src/kimi/kimiRemoteLauncher.ts b/cli/src/kimi/kimiRemoteLauncher.ts index 36e67931..f0c4f35f 100644 --- a/cli/src/kimi/kimiRemoteLauncher.ts +++ b/cli/src/kimi/kimiRemoteLauncher.ts @@ -253,6 +253,9 @@ class KimiRemoteLauncher extends RemoteLauncherBase { case 'error': this.messageBuffer.addMessage(message.message, 'status'); break; + case 'generated_image': + this.messageBuffer.addMessage(`Generated image: ${message.fileName}`, 'assistant'); + break; case 'turn_complete': this.messageBuffer.addMessage('Turn complete', 'status'); break; diff --git a/cli/src/kimi/utils/kimiBackend.ts b/cli/src/kimi/utils/kimiBackend.ts index ed4ce569..2604e978 100644 --- a/cli/src/kimi/utils/kimiBackend.ts +++ b/cli/src/kimi/utils/kimiBackend.ts @@ -21,6 +21,7 @@ export function createKimiBackend(): AcpSdkBackend { return new AcpSdkBackend({ command: 'kimi', args: ['acp'], - env: filterEnv(process.env) + env: filterEnv(process.env), + flavor: 'kimi', }); } diff --git a/cli/src/modules/common/displayImagePrompt.ts b/cli/src/modules/common/displayImagePrompt.ts new file mode 100644 index 00000000..5445a3f2 --- /dev/null +++ b/cli/src/modules/common/displayImagePrompt.ts @@ -0,0 +1,53 @@ +import { trimIdent } from '@/utils/trimIdent'; + +/** + * Shared display_image MCP tool hints — one export per tool naming convention. + * Inject into flavor system prompts and first-prompt bridge instructions. + */ +export const DISPLAY_IMAGE_PROMPT_CLAUDE = trimIdent(` + When you create or find a local image file that the user should see, call the tool "mcp__hapi__display_image" with the image path so HAPI can show it inline. +`); + +export const DISPLAY_IMAGE_PROMPT_CODEX = trimIdent(` + When you create or find a local image file that the user should see, call functions.hapi__display_image with the image path. If that exact tool name is unavailable, use an equivalent alias such as hapi__display_image, mcp__hapi__display_image, or hapi_display_image. +`); + +export const DISPLAY_IMAGE_PROMPT_HAPI_MCP = trimIdent(` + When you create or find a local image file that the user should see, call the tool "hapi_display_image" with the image path so HAPI can show it inline. If that exact tool name is unavailable, use an equivalent alias such as display_image or mcp__hapi__display_image. +`); + +export const DISPLAY_VIDEO_PROMPT_CLAUDE = trimIdent(` + When you create or find a local mp4 or webm recording the user should see, call the tool "mcp__hapi__display_video" with the file path so HAPI can show it inline. +`); + +export const DISPLAY_VIDEO_PROMPT_CODEX = trimIdent(` + When you create or find a local mp4 or webm file the user should see, call functions.hapi__display_video with the file path. If that exact tool name is unavailable, use an equivalent alias such as hapi__display_video, mcp__hapi__display_video, or hapi_display_video. +`); + +export const DISPLAY_VIDEO_PROMPT_HAPI_MCP = trimIdent(` + When you create or find a local mp4 or webm recording the user should see, call the tool "hapi_display_video" with the file path so HAPI can show it inline. If that exact tool name is unavailable, use an equivalent alias such as display_video or mcp__hapi__display_video. +`); + +export const DISPLAY_IMAGE_PROMPT_CURSOR = trimIdent(` + When you create or find a local image file that the user should see, call the tool "display_image" with the absolute filesystem path so HAPI can show it inline. +`); + +export const DISPLAY_VIDEO_PROMPT_CURSOR = trimIdent(` + When you create or find a local mp4 or webm recording the user should see, call the tool "display_video" with the absolute filesystem path so HAPI can show it inline. +`); + +export const DISPLAY_MEDIA_PROMPT_CLAUDE = trimIdent(` + When you create or find a local audio file or other non-image file that the user should receive, call the tool "mcp__hapi__display_media" with the file path so HAPI can show a player or download card. +`); + +export const DISPLAY_MEDIA_PROMPT_CODEX = trimIdent(` + When you create or find a local audio file or other non-image file that the user should receive, call functions.hapi__display_media with the file path. If that exact tool name is unavailable, use an equivalent alias such as hapi__display_media, mcp__hapi__display_media, or hapi_display_media. +`); + +export const DISPLAY_MEDIA_PROMPT_HAPI_MCP = trimIdent(` + When you create or find a local audio file or other non-image file that the user should receive, call the tool "hapi_display_media" with the file path. If that exact tool name is unavailable, use an equivalent alias such as display_media or mcp__hapi__display_media. +`); + +export const DISPLAY_MEDIA_PROMPT_CURSOR = trimIdent(` + When you create or find a local audio file or other non-image file that the user should receive, call the tool "display_media" with the absolute filesystem path so HAPI can show a player or download card. +`); diff --git a/cli/src/modules/common/generatedImages.test.ts b/cli/src/modules/common/generatedImages.test.ts index 471539e4..989a6d8d 100644 --- a/cli/src/modules/common/generatedImages.test.ts +++ b/cli/src/modules/common/generatedImages.test.ts @@ -1,5 +1,8 @@ import { describe, expect, it } from 'vitest' -import { clearGeneratedImages, detectImageMimeType, getGeneratedImage, registerGeneratedImage } from './generatedImages' +import { mkdirSync, writeFileSync } from 'node:fs' +import { join } from 'node:path' +import { tmpdir } from 'node:os' +import { clearGeneratedImages, decodeGeneratedImageBase64, detectAudioMimeType, detectDisplayMediaMimeType, detectImageMimeType, detectVideoMimeType, getGeneratedImage, MAX_GENERATED_IMAGE_BASE64_CHARS, MAX_GENERATED_IMAGE_BYTES, readBoundedRegularFile, registerGeneratedImage, registerGeneratedImageFromAcpBlock, registerGeneratedImageFromPath, safeAcpFileName } from './generatedImages' describe('generatedImages', () => { it('detects supported image MIME types from file bytes', () => { @@ -10,6 +13,74 @@ describe('generatedImages', () => { expect(detectImageMimeType(Buffer.from([0x00, 0x00, 0x00, 0x1c, 0x66, 0x74, 0x79, 0x70, 0x61, 0x76, 0x69, 0x66]))).toBe('image/avif') }) + it('detects supported video MIME types from file bytes', () => { + expect(detectVideoMimeType(Buffer.from([0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x69, 0x73, 0x6f, 0x6d]))).toBe('video/mp4') + // EBML + DocType "webm" (0x4282 / VINT len 4 / "webm") + expect(detectVideoMimeType(Buffer.from([ + 0x1a, 0x45, 0xdf, 0xa3, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1f, + 0x42, 0x82, 0x84, 0x77, 0x65, 0x62, 0x6d, + ]))).toBe('video/webm') + // Bare EBML magic (or Matroska DocType) must not claim video/webm + expect(detectVideoMimeType(Buffer.from([0x1a, 0x45, 0xdf, 0xa3]))).toBeNull() + expect(detectVideoMimeType(Buffer.from([ + 0x1a, 0x45, 0xdf, 0xa3, + 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x1f, + 0x42, 0x82, 0x88, 0x6d, 0x61, 0x74, 0x72, 0x6f, 0x73, 0x6b, 0x61, + ]))).toBeNull() + expect(detectVideoMimeType(Buffer.from([0x00, 0x00, 0x00, 0x1c, 0x66, 0x74, 0x79, 0x70, 0x61, 0x76, 0x69, 0x66]))).toBeNull() + // HEIC/HEIF uses ftyp but is not a supported inline video container + expect(detectVideoMimeType(Buffer.from([0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x68, 0x65, 0x69, 0x63]))).toBeNull() + expect(detectVideoMimeType(Buffer.from([0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x6d, 0x69, 0x66, 0x31]))).toBeNull() + }) + + it('detects supported audio MIME types from file bytes', () => { + expect(detectAudioMimeType(Buffer.from('RIFFxxxxWAVE'))).toBe('audio/wav') + expect(detectAudioMimeType(Buffer.from('fLaC'))).toBe('audio/flac') + expect(detectAudioMimeType(Buffer.concat([Buffer.from('OggS'), Buffer.alloc(24), Buffer.from('OpusHead')]))).toBe('audio/ogg') + expect(detectAudioMimeType(Buffer.from('ID3'))).toBe('audio/mpeg') + expect(detectAudioMimeType(Buffer.from([0xff, 0xfb, 0x90]))).toBe('audio/mpeg') + expect(detectAudioMimeType(Buffer.from([0xff, 0xe0, 0x00]))).toBeNull() + expect(detectAudioMimeType(Buffer.from([0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x4d, 0x34, 0x41, 0x20]))).toBe('audio/mp4') + }) + + it('falls back to a download-safe MIME type for unknown files', () => { + expect(detectDisplayMediaMimeType(Buffer.from('arbitrary file bytes'))).toBe('application/octet-stream') + }) + + it('uses ISO-BMFF track handlers to distinguish audio-only and video files', () => { + const box = (type: string, payload: Buffer) => { + const header = Buffer.alloc(8) + header.writeUInt32BE(header.length + payload.length) + header.write(type, 4, 4, 'ascii') + return Buffer.concat([header, payload]) + } + const ftypIsom = box('ftyp', Buffer.from('isom0000')) + const track = (handler: 'soun' | 'vide') => box('moov', box('trak', box('mdia', box('hdlr', Buffer.concat([ + Buffer.alloc(8), + Buffer.from(handler), + Buffer.alloc(4) + ]))))) + + expect(detectDisplayMediaMimeType(Buffer.concat([ftypIsom, track('soun')]))).toBe('audio/mp4') + expect(detectDisplayMediaMimeType(Buffer.concat([ftypIsom, track('vide')]))).toBe('video/mp4') + expect(detectDisplayMediaMimeType(ftypIsom)).toBe('application/octet-stream') + expect(detectDisplayMediaMimeType(Buffer.concat([ + ftypIsom, + box('mdat', Buffer.concat([Buffer.from([0, 0, 0, 24]), Buffer.from('hdlr'), Buffer.alloc(8), Buffer.from('soun')])) + ]))).toBe('application/octet-stream') + expect(detectDisplayMediaMimeType(Buffer.concat([ftypIsom, track('soun').subarray(0, -1)]))).toBe('application/octet-stream') + expect(detectDisplayMediaMimeType(Buffer.concat([ftypIsom, track('soun'), Buffer.alloc(1)]))).toBe('application/octet-stream') + expect(detectDisplayMediaMimeType(Buffer.concat([ + ftypIsom, + box('moov', Buffer.concat([box('trak', box('mdia', box('hdlr', Buffer.concat([ + Buffer.alloc(8), + Buffer.from('soun'), + Buffer.alloc(4) + ])))), Buffer.alloc(1)])) + ]))).toBe('application/octet-stream') + }) + it('rejects non-image bytes even if the path has an image extension', () => { expect(detectImageMimeType(Buffer.from('not really a png'))).toBeNull() }) @@ -26,6 +97,25 @@ describe('generatedImages', () => { clearGeneratedImages() }) + it('stores audio and generic download media', () => { + const audio = registerGeneratedImage({ + id: 'test-audio', + path: '/tmp/example.wav', + mimeType: 'audio/wav', + bytes: Buffer.from('RIFFxxxxWAVE') + }) + const file = registerGeneratedImage({ + id: 'test-file', + path: '/tmp/example.bin', + mimeType: 'application/octet-stream', + bytes: Buffer.from('file') + }) + + expect(audio.mimeType).toBe('audio/wav') + expect(file.mimeType).toBe('application/octet-stream') + clearGeneratedImages() + }) + it('snapshots image bytes at registration time', () => { const source = Buffer.from('original image bytes') const image = registerGeneratedImage({ @@ -47,7 +137,7 @@ describe('generatedImages', () => { path: '/tmp/large.png', mimeType: 'image/png', bytes: new Uint8Array(25 * 1024 * 1024 + 1) - })).toThrow('Image is too large to display inline') + })).toThrow('File is too large to display inline') clearGeneratedImages() }) @@ -67,4 +157,94 @@ describe('generatedImages', () => { clearGeneratedImages() }) + it('registers images from ACP base64 image blocks after MIME sniffing', async () => { + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x00]) + const image = await registerGeneratedImageFromAcpBlock({ + type: 'image', + mimeType: 'image/png', + data: pngHeader.toString('base64') + }) + + expect(image?.mimeType).toBe('image/png') + expect(getGeneratedImage(image!.id)?.content.subarray(0, 8)).toEqual(pngHeader.subarray(0, 8)) + clearGeneratedImages() + }) + + it('rejects oversized base64 before allocating a decoded buffer', async () => { + const oversized = 'A'.repeat(MAX_GENERATED_IMAGE_BASE64_CHARS + 1) + expect(decodeGeneratedImageBase64(oversized)).toBeNull() + await expect(registerGeneratedImageFromAcpBlock({ + type: 'image', + mimeType: 'image/png', + data: oversized, + })).resolves.toBeNull() + }) + + it('ignores URI-only ACP image blocks that would read local disk without a permission prompt', async () => { + const dir = join(tmpdir(), `hapi-acp-uri-only-${Date.now()}`) + mkdirSync(dir, { recursive: true }) + const path = join(dir, 'secret.png') + writeFileSync(path, Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00])) + + await expect(registerGeneratedImageFromAcpBlock({ + type: 'image', + uri: `file://${path}` + })).resolves.toBeNull() + + await expect(registerGeneratedImageFromAcpBlock({ + type: 'image', + url: path + })).resolves.toBeNull() + }) + + it('registers images from local file paths in ACP uri blocks', async () => { + const dir = join(tmpdir(), `hapi-acp-image-${Date.now()}`) + mkdirSync(dir, { recursive: true }) + const path = join(dir, 'inline.png') + const bytes = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00]) + writeFileSync(path, bytes) + + const image = await registerGeneratedImageFromPath({ path }) + expect(image?.mimeType).toBe('image/png') + clearGeneratedImages() + }) + + it('registers mp4 from local file paths after MIME sniffing', async () => { + const dir = join(tmpdir(), `hapi-inline-mp4-${Date.now()}`) + mkdirSync(dir, { recursive: true }) + const path = join(dir, 'inline.mp4') + const bytes = Buffer.from([0x00, 0x00, 0x00, 0x18, 0x66, 0x74, 0x79, 0x70, 0x69, 0x73, 0x6f, 0x6d]) + writeFileSync(path, bytes) + + const video = await registerGeneratedImageFromPath({ path }) + expect(video?.mimeType).toBe('video/mp4') + clearGeneratedImages() + }) + + it('readBoundedRegularFile rejects oversize files without allocating the full path size', async () => { + const dir = join(tmpdir(), `hapi-bounded-read-${Date.now()}`) + mkdirSync(dir, { recursive: true }) + const path = join(dir, 'big.bin') + writeFileSync(path, Buffer.alloc(1024, 0xab)) + + await expect(readBoundedRegularFile(path, 512)).rejects.toThrow(/too large/i) + await expect(readBoundedRegularFile(path, MAX_GENERATED_IMAGE_BYTES)).resolves.toHaveLength(1024) + }) + + it('safeAcpFileName rejects data URIs and strips signed URL query secrets', async () => { + expect(safeAcpFileName('data:image/png;base64,AAAA')).toBeNull() + expect(safeAcpFileName('https://cdn.example/img/shot.png?token=secret')).toBe('shot.png') + expect(safeAcpFileName('file:///tmp/photos/icon.png')).toBe('icon.png') + + const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a, 0x00, 0x00, 0x00, 0x00]) + const image = await registerGeneratedImageFromAcpBlock({ + type: 'image', + mimeType: 'image/png', + data: pngHeader.toString('base64'), + uri: 'data:image/png;base64,' + pngHeader.toString('base64'), + }) + expect(image?.fileName).toMatch(/^generated-/) + expect(image?.fileName.startsWith('data:')).toBe(false) + clearGeneratedImages() + }) }) diff --git a/cli/src/modules/common/generatedImages.ts b/cli/src/modules/common/generatedImages.ts index fdf6cb94..7d268d5c 100644 --- a/cli/src/modules/common/generatedImages.ts +++ b/cli/src/modules/common/generatedImages.ts @@ -1,4 +1,41 @@ -import { basename } from 'path' +import { basename } from 'node:path' +import { fileURLToPath } from 'node:url' +import { open } from 'node:fs/promises' +import { randomUUID } from 'node:crypto' +import { asString, isObject } from '@hapi/protocol' + +/** + * Read a regular file through one open descriptor: size is taken from the fd + * (not a separate pathname `lstat`), then the buffer is allocated to that size + * only. Rejects if the file grows/shrinks while reading so a TOCTOU swap cannot + * force an unbounded `readFile` allocation. + */ +export async function readBoundedRegularFile(path: string, maxBytes: number): Promise { + const handle = await open(path, 'r') + try { + const info = await handle.stat() + if (!info.isFile()) { + throw new Error('Path is not a regular file') + } + if (info.size > maxBytes) { + throw new Error('File is too large to display inline') + } + const bytes = Buffer.alloc(info.size) + for (let offset = 0; offset < bytes.length;) { + const { bytesRead } = await handle.read(bytes, offset, bytes.length - offset, offset) + if (bytesRead === 0) { + throw new Error('File changed while reading') + } + offset += bytesRead + } + if ((await handle.stat()).size !== info.size) { + throw new Error('File changed while reading') + } + return bytes + } finally { + await handle.close() + } +} export type GeneratedImageMetadata = { id: string @@ -8,10 +45,24 @@ export type GeneratedImageMetadata = { createdAt: number } -const MAX_GENERATED_IMAGE_BYTES = 25 * 1024 * 1024 +export const MAX_GENERATED_IMAGE_BYTES = 25 * 1024 * 1024 +/** Reject base64 strings that cannot fit under MAX_GENERATED_IMAGE_BYTES once decoded (+ padding). */ +export const MAX_GENERATED_IMAGE_BASE64_CHARS = Math.ceil(MAX_GENERATED_IMAGE_BYTES * 4 / 3) + 4 const MAX_GENERATED_IMAGE_TOTAL_BYTES = 100 * 1024 * 1024 const MAX_GENERATED_IMAGE_COUNT = 100 +/** Decode inline media base64 only after a cheap length gate (avoids huge Buffer allocations). */ +export function decodeGeneratedImageBase64(data: string): Buffer | null { + if (data.length > MAX_GENERATED_IMAGE_BASE64_CHARS) { + return null + } + const bytes = Buffer.from(data, 'base64') + if (bytes.byteLength > MAX_GENERATED_IMAGE_BYTES) { + return null + } + return bytes +} + const generatedImages = new Map() let generatedImageBytes = 0 @@ -55,6 +106,193 @@ export function detectImageMimeType(bytes: Uint8Array): string | null { return null } +const MP4_FTYP_BRANDS = new Set([ + 'isom', + 'iso2', + 'iso3', + 'iso4', + 'iso5', + 'iso6', + 'mp41', + 'mp42', + 'mp71', + 'avc1', + 'avc3', + 'hev1', + 'hvc1', + 'mmp4', + 'dash', + 'msnv', + 'ndas', + 'ndsc', + 'ndsh', + 'ndsm', + 'ndsp', + 'ndss', + 'ndxc', + 'ndxh', + 'ndxm', + 'ndxp', + 'ndxs', +]) + +/** EBML DocType (element 0x4282) from the first header bytes; null if absent/unreadable. */ +function readEbmlDocType(bytes: Uint8Array): string | null { + const limit = Math.min(bytes.length, 128) + for (let i = 4; i + 3 < limit; i += 1) { + if (bytes[i] !== 0x42 || bytes[i + 1] !== 0x82) { + continue + } + const sizeByte = bytes[i + 2] + // Single-byte VINT: high bit set, length in low 7 bits. + if ((sizeByte & 0x80) === 0) { + continue + } + const len = sizeByte & 0x7f + if (len === 0 || i + 3 + len > limit) { + continue + } + return ascii(bytes, i + 3, i + 3 + len) + } + return null +} + +export function detectVideoMimeType(bytes: Uint8Array): string | null { + if (bytes.length >= 12 && ascii(bytes, 4, 8) === 'ftyp') { + const brand = ascii(bytes, 8, 12) + if (MP4_FTYP_BRANDS.has(brand)) { + return 'video/mp4' + } + return null + } + + if (bytes.length >= 4 + && bytes[0] === 0x1a + && bytes[1] === 0x45 + && bytes[2] === 0xdf + && bytes[3] === 0xa3) { + // EBML is shared by WebM and Matroska/MKV — only accept DocType webm. + if (readEbmlDocType(bytes) === 'webm') { + return 'video/webm' + } + return null + } + + return null +} + +export function detectAudioMimeType(bytes: Uint8Array): string | null { + if (bytes.length >= 12 && ascii(bytes, 0, 4) === 'RIFF' && ascii(bytes, 8, 12) === 'WAVE') { + return 'audio/wav' + } + + if (bytes.length >= 4 && ascii(bytes, 0, 4) === 'fLaC') { + return 'audio/flac' + } + + if (bytes.length >= 32 && ascii(bytes, 0, 4) === 'OggS') { + const probe = ascii(bytes, 0, Math.min(bytes.length, 96)) + if (probe.includes('OpusHead') || probe.includes('vorbis') || probe.includes('Speex')) { + return 'audio/ogg' + } + } + + if (bytes.length >= 3 && ascii(bytes, 0, 3) === 'ID3') { + return 'audio/mpeg' + } + + if (bytes.length >= 3 + && bytes[0] === 0xff + && (bytes[1] & 0xe0) === 0xe0 + && (bytes[1] & 0x18) !== 0x08 + && (bytes[1] & 0x06) !== 0 + && (bytes[2] & 0xf0) !== 0 + && (bytes[2] & 0xf0) !== 0xf0 + && (bytes[2] & 0x0c) !== 0x0c) { + return 'audio/mpeg' + } + + if (bytes.length >= 12 && ascii(bytes, 4, 8) === 'ftyp') { + const brand = ascii(bytes, 8, 12) + if (brand === 'M4A ' || brand === 'M4B ' || brand === 'M4P ') { + return 'audio/mp4' + } + } + + return null +} + +function detectIsoBmffTrackKind(bytes: Uint8Array): 'audio' | 'video' | null { + const buffer = Buffer.from(bytes.buffer, bytes.byteOffset, bytes.byteLength) + let hasAudio = false + let hasVideo = false + + function visitBoxes(start: number, end: number, parent: 'root' | 'moov' | 'trak' | 'mdia'): boolean { + let offset = start + for (; offset + 8 <= end;) { + const size32 = buffer.readUInt32BE(offset) + const type = ascii(bytes, offset + 4, offset + 8) + let headerSize = 8 + let boxSize: number + if (size32 === 0) { + boxSize = end - offset + } else if (size32 === 1) { + if (offset + 16 > end) return false + const extendedSize = buffer.readBigUInt64BE(offset + 8) + if (extendedSize > BigInt(Number.MAX_SAFE_INTEGER)) return false + headerSize = 16 + boxSize = Number(extendedSize) + } else { + boxSize = size32 + } + if (boxSize < headerSize || boxSize > end - offset) return false + + const payloadStart = offset + headerSize + const boxEnd = offset + boxSize + if (parent === 'root' && type === 'moov') { + if (!visitBoxes(payloadStart, boxEnd, 'moov')) return false + } else if (parent === 'moov' && type === 'trak') { + if (!visitBoxes(payloadStart, boxEnd, 'trak')) return false + } else if (parent === 'trak' && type === 'mdia') { + if (!visitBoxes(payloadStart, boxEnd, 'mdia')) return false + } else if (parent === 'mdia' && type === 'hdlr') { + if (boxSize < headerSize + 12) return false + const handlerType = ascii(bytes, payloadStart + 8, payloadStart + 12) + if (handlerType === 'soun') hasAudio = true + if (handlerType === 'vide') hasVideo = true + } + + offset = boxEnd + } + return offset === end + } + + if (!visitBoxes(0, bytes.length, 'root')) return null + if (hasVideo) return 'video' + if (hasAudio) return 'audio' + return null +} + +export function isInlineMediaMimeType(mimeType: string): boolean { + return mimeType.startsWith('image/') || mimeType.startsWith('video/') || mimeType.startsWith('audio/') +} + +export function detectDisplayMediaMimeType(bytes: Uint8Array): string { + const imageMimeType = detectImageMimeType(bytes) + if (imageMimeType) return imageMimeType + + if (bytes.length >= 12 && ascii(bytes, 4, 8) === 'ftyp') { + const trackKind = detectIsoBmffTrackKind(bytes) + if (trackKind === 'video') return 'video/mp4' + if (trackKind === 'audio') return 'audio/mp4' + return detectAudioMimeType(bytes) ?? 'application/octet-stream' + } + + return detectVideoMimeType(bytes) + ?? detectAudioMimeType(bytes) + ?? 'application/octet-stream' +} + function ascii(bytes: Uint8Array, start: number, end: number): string { return String.fromCharCode(...bytes.subarray(start, end)) } @@ -62,7 +300,11 @@ function ascii(bytes: Uint8Array, start: number, end: number): string { export function registerGeneratedImage(args: { id: string; path: string; mimeType: string; bytes: Uint8Array; fileName?: string | null }): GeneratedImageMetadata { const content = Buffer.from(args.bytes) if (content.byteLength > MAX_GENERATED_IMAGE_BYTES) { - throw new Error('Image is too large to display inline') + throw new Error('File is too large to display inline') + } + + if (!isInlineMediaMimeType(args.mimeType) && args.mimeType !== 'application/octet-stream') { + throw new Error('Unsupported generated media MIME type') } const previous = generatedImages.get(args.id) @@ -105,3 +347,98 @@ export function clearGeneratedImages(): void { generatedImages.clear() generatedImageBytes = 0 } + +export async function registerGeneratedImageFromPath(args: { + id?: string + path: string + fileName?: string | null +}): Promise { + try { + const bytes = await readBoundedRegularFile(args.path, MAX_GENERATED_IMAGE_BYTES) + const mimeType = detectImageMimeType(bytes) ?? detectVideoMimeType(bytes) + if (!mimeType) { + throw new Error('Unsupported inline media content') + } + return registerGeneratedImage({ + id: args.id ?? randomUUID(), + path: args.path, + fileName: args.fileName, + mimeType, + bytes + }) + } catch { + return null + } +} + +function parseAcpImageUri(uri: string): string | null { + if (uri.startsWith('file://')) { + try { + return fileURLToPath(uri) + } catch { + return null + } + } + if (/^https?:\/\//i.test(uri)) { + return null + } + return uri +} + +/** Bound safe display name from an ACP uri/url — never reuse raw data/signed URLs. */ +export function safeAcpFileName(uri: string | undefined | null): string | null { + if (!uri) return null + try { + if (uri.startsWith('file://')) { + return basename(fileURLToPath(uri)).slice(0, 255) || null + } + if (/^https?:\/\//i.test(uri)) { + const name = basename(new URL(uri).pathname) + return name.slice(0, 255) || null + } + // Reject scheme-bearing forms (data:, blob:, etc.) — only bare paths. + if (!uri.includes(':')) { + return basename(uri).slice(0, 255) || null + } + } catch { + return null + } + return null +} + +export async function registerGeneratedImageFromAcpBlock(block: unknown): Promise { + if (!isObject(block) || block.type !== 'image') { + return null + } + + const data = asString(block.data) + const declaredMimeType = asString(block.mimeType ?? block.mime_type) + const uri = asString(block.uri ?? block.url) + + if (data) { + const bytes = decodeGeneratedImageBase64(data) + if (!bytes) { + return null + } + const sniffedMimeType = detectImageMimeType(bytes) + if (!sniffedMimeType) { + return null + } + if (declaredMimeType && declaredMimeType !== sniffedMimeType) { + return null + } + const localPath = uri ? parseAcpImageUri(uri) : null + const fileName = safeAcpFileName(uri) ?? `generated-${randomUUID()}.png` + return registerGeneratedImage({ + id: randomUUID(), + path: localPath ?? fileName, + fileName, + mimeType: sniffedMimeType, + bytes + }) + } + + // URI-only ACP image blocks are not permission-gated. Local-path display must + // go through display_image / display_video / display_media MCP tools (approval_mode: prompt). + return null +} diff --git a/cli/src/modules/common/hapiMcpBridgePrompt.ts b/cli/src/modules/common/hapiMcpBridgePrompt.ts new file mode 100644 index 00000000..3017c362 --- /dev/null +++ b/cli/src/modules/common/hapiMcpBridgePrompt.ts @@ -0,0 +1,30 @@ +import { trimIdent } from '@/utils/trimIdent'; +import { + DISPLAY_IMAGE_PROMPT_HAPI_MCP, + DISPLAY_MEDIA_PROMPT_HAPI_MCP, + DISPLAY_VIDEO_PROMPT_HAPI_MCP, +} from './displayImagePrompt'; + +/** Shell fallback for `hapi doctor inline-media` only — not injected into agent prompts. */ +export const INLINE_MEDIA_SHELL_FALLBACK = trimIdent(` + If display_image / display_video / display_media tools are not in your tool list, inline media via shell from the HAPI repo (needs bun + @modelcontextprotocol/sdk): + cd && bun scripts/tooling/hapi-display-image.mjs "title" + Use the HAPI session uuid prefix from the web URL /sessions/ (first 8 chars), not cursorSessionId or other agent-native ids. + Run hapi doctor inline-media to list active bridges and copy the exact command. +`); + +/** + * Title + display_image / display_video / display_media instructions for OpenCode first-prompt / + * hapi-instructions.md. Cursor uses native MCP overlay + tool descriptions instead + * (no user-turn prepend — that path was prompt-taint). + */ +export const HAPI_MCP_TITLE_INSTRUCTION = trimIdent(` + Use the title tool sparingly. For a new chat, call the tool "hapi_change_title" once after the user's initial request is clear, and set a concise task title. Do not rename the chat for routine progress, substeps, implementation details, or a slightly better wording. Rename only when the user's primary objective changes substantially and the existing title would be misleading. +`); + +export const HAPI_MCP_BRIDGE_PROMPT = trimIdent(` + ${HAPI_MCP_TITLE_INSTRUCTION} + ${DISPLAY_IMAGE_PROMPT_HAPI_MCP} + ${DISPLAY_VIDEO_PROMPT_HAPI_MCP} + ${DISPLAY_MEDIA_PROMPT_HAPI_MCP} +`); diff --git a/cli/src/modules/common/inlineMediaSource.test.ts b/cli/src/modules/common/inlineMediaSource.test.ts new file mode 100644 index 00000000..abe470e5 --- /dev/null +++ b/cli/src/modules/common/inlineMediaSource.test.ts @@ -0,0 +1,22 @@ +import { describe, expect, it } from 'vitest'; +import { inlineMediaSourceFromWire } from './inlineMediaSource'; + +describe('inlineMediaSourceFromWire', () => { + it('parses ingress and snake_case tool fields', () => { + expect(inlineMediaSourceFromWire({ + ingress: 'mcp', + flavor: 'cursor', + tool_call_id: 'call-1', + tool_name: 'display_video', + })).toEqual({ + ingress: 'mcp', + flavor: 'cursor', + toolCallId: 'call-1', + toolName: 'display_video', + }); + }); + + it('accepts legacy path alias for ingress', () => { + expect(inlineMediaSourceFromWire({ path: 'acp' })).toEqual({ ingress: 'acp' }); + }); +}); diff --git a/cli/src/modules/common/inlineMediaSource.ts b/cli/src/modules/common/inlineMediaSource.ts new file mode 100644 index 00000000..1c095a2c --- /dev/null +++ b/cli/src/modules/common/inlineMediaSource.ts @@ -0,0 +1,28 @@ +/** How generated media entered the session (v1 provenance seed for #956 / artifact follow-up). */ +export type InlineMediaIngress = 'mcp' | 'acp' | 'tool_result'; + +export type InlineMediaSource = { + ingress: InlineMediaIngress; + flavor?: string; + toolCallId?: string; + toolName?: string; +}; + +export function inlineMediaSourceFromWire(value: unknown): InlineMediaSource | undefined { + if (!value || typeof value !== 'object' || Array.isArray(value)) return undefined; + const record = value as Record; + const ingress = record.ingress ?? record.path; + if (ingress !== 'mcp' && ingress !== 'acp' && ingress !== 'tool_result') return undefined; + const flavor = typeof record.flavor === 'string' ? record.flavor : undefined; + const toolCallId = typeof record.toolCallId === 'string' + ? record.toolCallId + : typeof record.tool_call_id === 'string' + ? record.tool_call_id + : undefined; + const toolName = typeof record.toolName === 'string' + ? record.toolName + : typeof record.tool_name === 'string' + ? record.tool_name + : undefined; + return { ingress, flavor, toolCallId, toolName }; +} diff --git a/cli/src/modules/common/skillLookupInstruction.ts b/cli/src/modules/common/skillLookupInstruction.ts index aa04d33f..8cf027e2 100644 --- a/cli/src/modules/common/skillLookupInstruction.ts +++ b/cli/src/modules/common/skillLookupInstruction.ts @@ -4,7 +4,7 @@ * prompt injection on Cursor ACP and similar remotes (tiann/hapi#1095). * * Cursor / Kimi / generic ACP rely on the `skill_lookup` MCP tool description - * (and Cursor's native `.cursor/mcp.json` overlay where session/new mcpServers + * (and Cursor's native `~/.cursor/mcp.json` overlay where session/new mcpServers * are ignored) instead of a user-message prepend. */ export const SKILL_LOOKUP_INSTRUCTION = diff --git a/cli/src/opencode/opencodeRemoteLauncher.ts b/cli/src/opencode/opencodeRemoteLauncher.ts index 12e91c4d..05cf4673 100644 --- a/cli/src/opencode/opencodeRemoteLauncher.ts +++ b/cli/src/opencode/opencodeRemoteLauncher.ts @@ -571,7 +571,7 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase { const promptContent: PromptContent[] = [{ type: 'text', - text: messageText + text: messageText, }]; this.stallErrorReportedForPrompt = false; @@ -906,6 +906,9 @@ class OpencodeRemoteLauncher extends RemoteLauncherBase { case 'error': this.messageBuffer.addMessage(message.message, 'status'); break; + case 'generated_image': + this.messageBuffer.addMessage(`Generated image: ${message.fileName}`, 'assistant'); + break; case 'turn_complete': this.messageBuffer.addMessage('Turn complete', 'status'); break; diff --git a/cli/src/opencode/utils/opencodeBackend.ts b/cli/src/opencode/utils/opencodeBackend.ts index 45fc774b..9d8e5c3d 100644 --- a/cli/src/opencode/utils/opencodeBackend.ts +++ b/cli/src/opencode/utils/opencodeBackend.ts @@ -64,6 +64,7 @@ export function createOpencodeBackend(opts: { command: 'opencode', args, env: filterEnv(env), - textChunkMode: 'delta' + textChunkMode: 'delta', + flavor: 'opencode', }); } diff --git a/cli/src/opencode/utils/systemPrompt.ts b/cli/src/opencode/utils/systemPrompt.ts index f57a1230..b1838d33 100644 --- a/cli/src/opencode/utils/systemPrompt.ts +++ b/cli/src/opencode/utils/systemPrompt.ts @@ -1,21 +1,26 @@ /** - * OpenCode-specific system prompt for change_title tool. + * OpenCode-specific system prompt for hapi MCP tools (change_title, display_image, display_video, display_media). * * OpenCode exposes MCP tools with the naming pattern: _ - * The hapi MCP server exposes `change_title`, so it's called as `hapi_change_title`. + * The hapi MCP server exposes `change_title`, `display_image`, `display_video`, and `display_media`. */ import { trimIdent } from '@/utils/trimIdent'; import { buildSessionCitationSteerInstruction } from '@hapi/protocol/sessionCitation'; +import { HAPI_MCP_BRIDGE_PROMPT } from '@/modules/common/hapiMcpBridgePrompt'; +import { + DISPLAY_IMAGE_PROMPT_HAPI_MCP, + DISPLAY_MEDIA_PROMPT_HAPI_MCP, + DISPLAY_VIDEO_PROMPT_HAPI_MCP, +} from '@/modules/common/displayImagePrompt'; import { SKILL_LOOKUP_INSTRUCTION } from '@/modules/common/skillLookupInstruction'; import { withSessionSummaryInstruction } from '@/modules/common/sessionSummaryInstruction'; /** - * Title instruction for OpenCode to call the hapi MCP tool. + * Title and display_image / display_video / display_media instructions for OpenCode to call the hapi MCP tools. */ export const TITLE_INSTRUCTION = trimIdent(` - Use the title tool sparingly. For a new chat, call the tool "hapi_change_title" once after the user's initial request is clear, and set a concise task title. Do not rename the chat for routine progress, substeps, implementation details, or a slightly better wording. Rename only when the user's primary objective changes substantially and the existing title would be misleading. - When you create or find a local image file that the user should see, call the tool "hapi_display_image" with the image path so HAPI can show it inline. + ${HAPI_MCP_BRIDGE_PROMPT} ${buildSessionCitationSteerInstruction({ inspectTool: 'hapi_inspect_peer', pingTool: 'hapi_ping_peer', @@ -33,7 +38,9 @@ export function getTitleInstruction(env: NodeJS.ProcessEnv = process.env): strin * advertise only the MCP tools that remain available to the model. */ export const OPENCODE_NATIVE_TOOL_INSTRUCTION = trimIdent(` - When you create or find a local image file that the user should see, call the tool "hapi_display_image" with the image path so HAPI can show it inline. + ${DISPLAY_IMAGE_PROMPT_HAPI_MCP} + ${DISPLAY_VIDEO_PROMPT_HAPI_MCP} + ${DISPLAY_MEDIA_PROMPT_HAPI_MCP} ${buildSessionCitationSteerInstruction({ inspectTool: 'hapi_inspect_peer', pingTool: 'hapi_ping_peer', diff --git a/cli/src/ui/doctorInlineMedia.test.ts b/cli/src/ui/doctorInlineMedia.test.ts new file mode 100644 index 00000000..421265f8 --- /dev/null +++ b/cli/src/ui/doctorInlineMedia.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from 'vitest' +import { + formatInlineMediaCommand, + inlineMediaHelperScriptPath, + shellSingleQuote, +} from './doctorInlineMedia' + +describe('doctorInlineMedia', () => { + it('formatInlineMediaCommand uses repo scripts path', () => { + const script = inlineMediaHelperScriptPath() + expect(formatInlineMediaCommand(script, '341fe421')).toContain( + "bun scripts/tooling/hapi-display-image.mjs '341fe421'" + ) + }) + + it('formatInlineMediaCommand shell-quotes paths with spaces and metacharacters', () => { + const cmd = formatInlineMediaCommand( + '/tmp/my repo/cli/src/ui/doctorInlineMedia.ts', + 'abc12345', + '/tmp/my pics/shot.png', + ) + // scriptPath is cli/src/ui/... → repo root is three levels up (cli) + expect(cmd).toContain("cd '/tmp/my repo/cli'") + expect(cmd).toContain("'abc12345'") + expect(cmd).toContain("'/tmp/my pics/shot.png'") + }) + + it('shellSingleQuote escapes embedded single quotes for POSIX', () => { + expect(shellSingleQuote("it's")).toBe(`'it'"'"'s'`) + expect(shellSingleQuote('$(echo hi)')).toBe("'$(echo hi)'") + }) +}) diff --git a/cli/src/ui/doctorInlineMedia.ts b/cli/src/ui/doctorInlineMedia.ts new file mode 100644 index 00000000..5f3cb82a --- /dev/null +++ b/cli/src/ui/doctorInlineMedia.ts @@ -0,0 +1,263 @@ +/** + * Inline media bridge diagnostics (display_image / display_video / display_media + helper script). + */ + +import chalk from 'chalk' +import { existsSync } from 'node:fs' +import { join, resolve } from 'node:path' +import { configuration } from '@/configuration' +import { buildHubRequestHeaders } from '@/api/hubExtraHeaders' +import { readSettings } from '@/persistence' +import { projectPath } from '@/projectPath' +import { cursorHapiMcpServerId } from '@/cursor/utils/cursorMcpOverlay' + +export type InlineMediaDoctorCheck = { + ok: boolean + label: string + detail: string +} + +export type InlineMediaSessionBridge = { + id: string + prefix: string + flavor: string | null + hapiMcpUrl: string | null + listShowsMcpUrl: boolean + path: string | null + name: string | null +} + +function repoRootFromCli(): string { + return resolve(projectPath(), '..') +} + +export function inlineMediaHelperScriptPath(): string { + return join(repoRootFromCli(), 'scripts/tooling/hapi-display-image.mjs') +} + +function mcpSdkResolvable(): boolean { + const candidates = [ + join(projectPath(), 'node_modules/@modelcontextprotocol/sdk/package.json'), + join(repoRootFromCli(), 'node_modules/@modelcontextprotocol/sdk/package.json'), + ] + return candidates.some((p) => existsSync(p)) +} + +/** POSIX-safe single-quote wrapping (handles embedded quotes). */ +export function shellSingleQuote(value: string): string { + return `'${value.replaceAll("'", "'\"'\"'")}'` +} + +async function hubJwt(): Promise { + const settings = await readSettings() + const token = process.env.CLI_API_TOKEN ?? settings.cliApiToken + if (!token) { + return null + } + const res = await fetch(`${configuration.apiUrl}/api/auth`, { + method: 'POST', + headers: buildHubRequestHeaders({ 'Content-Type': 'application/json' }), + body: JSON.stringify({ accessToken: token }), + }) + if (!res.ok) { + return null + } + const body = (await res.json()) as { token?: string } + return body.token ?? null +} + +function asRecord(value: unknown): Record | null { + return value !== null && typeof value === 'object' ? value as Record : null +} + +function sessionDisplayName(metadata: Record | null): string | null { + if (!metadata) return null + const name = metadata.name + return typeof name === 'string' ? name : null +} + +export function formatInlineMediaCommand( + scriptPath: string, + sessionPrefix: string, + samplePath = '/absolute/path/to/image.png' +): string { + const scriptDir = resolve(scriptPath, '..', '..', '..') + const q = shellSingleQuote + return `cd ${q(scriptDir)} && bun scripts/tooling/hapi-display-image.mjs ${q(sessionPrefix)} ${q(samplePath)} "title"` +} + +export async function collectInlineMediaSessionBridges(jwt: string): Promise { + const listRes = await fetch(`${configuration.apiUrl}/api/sessions?limit=200`, { + headers: buildHubRequestHeaders({ Authorization: `Bearer ${jwt}` }), + }) + if (!listRes.ok) { + throw new Error(`sessions list failed: ${listRes.status}`) + } + const listBody = (await listRes.json()) as { sessions?: unknown[] } + const sessions = Array.isArray(listBody.sessions) ? listBody.sessions : [] + const active = sessions.filter((s) => asRecord(s)?.active === true) + + const bridges: InlineMediaSessionBridge[] = [] + for (const row of active) { + const summary = asRecord(row) + if (!summary || typeof summary.id !== 'string') continue + const listMeta = asRecord(summary.metadata) + const listMcp = listMeta && typeof listMeta.hapiMcpUrl === 'string' ? listMeta.hapiMcpUrl : null + + const detailRes = await fetch( + `${configuration.apiUrl}/api/sessions/${encodeURIComponent(summary.id)}`, + { headers: buildHubRequestHeaders({ Authorization: `Bearer ${jwt}` }) } + ) + if (!detailRes.ok) continue + const detailBody = (await detailRes.json()) as { session?: unknown } + const detailRow = asRecord(detailBody.session) ?? asRecord(detailBody) + const detailMeta = asRecord(detailRow?.metadata) + const detailMcp = detailMeta && typeof detailMeta.hapiMcpUrl === 'string' ? detailMeta.hapiMcpUrl : null + const flavor = detailMeta && typeof detailMeta.flavor === 'string' ? detailMeta.flavor : null + const path = detailMeta && typeof detailMeta.path === 'string' ? detailMeta.path : null + + bridges.push({ + id: summary.id, + prefix: summary.id.slice(0, 8), + flavor, + hapiMcpUrl: detailMcp, + listShowsMcpUrl: listMcp !== null, + path, + name: sessionDisplayName(detailMeta), + }) + } + return bridges +} + +export async function runDoctorInlineMedia(): Promise { + console.log(chalk.bold.cyan('\n🖼️ hapi inline media doctor\n')) + + const checks: InlineMediaDoctorCheck[] = [] + const scriptPath = inlineMediaHelperScriptPath() + const scriptExists = existsSync(scriptPath) + checks.push({ + ok: scriptExists, + label: 'Helper script (repo shell fallback)', + detail: scriptExists ? scriptPath : `missing: ${scriptPath} (optional outside source checkout)`, + }) + + const sdkOk = mcpSdkResolvable() + checks.push({ + ok: sdkOk, + label: '@modelcontextprotocol/sdk (repo shell fallback)', + detail: sdkOk ? 'resolvable from cli or repo root' : 'not found — optional outside source checkout', + }) + + const envSessionId = process.env.HAPI_SESSION_ID + if (envSessionId) { + checks.push({ + ok: true, + label: 'HAPI_SESSION_ID', + detail: envSessionId, + }) + } + + let jwt: string | null = null + try { + jwt = await hubJwt() + } catch { + jwt = null + } + checks.push({ + ok: jwt !== null, + label: 'Hub auth', + detail: jwt ? configuration.apiUrl : 'CLI_API_TOKEN missing or auth failed', + }) + + for (const check of checks) { + const mark = check.ok + ? chalk.green('✓') + : (check.label === 'Hub auth' ? chalk.red('✗') : chalk.yellow('○')) + console.log(`${mark} ${check.label}: ${chalk.gray(check.detail)}`) + } + + if (!jwt) { + console.log(chalk.red('\nCannot probe sessions without hub auth.\n')) + return 1 + } + + let bridges: InlineMediaSessionBridge[] = [] + try { + bridges = await collectInlineMediaSessionBridges(jwt) + } catch (error) { + const msg = error instanceof Error ? error.message : String(error) + console.log(chalk.red(`\n✗ Session probe failed: ${msg}\n`)) + return 1 + } + + const withBridge = bridges.filter((b) => b.hapiMcpUrl) + const listOmitsMcp = bridges.some((b) => b.hapiMcpUrl && !b.listShowsMcpUrl) + const shellFallbackAvailable = scriptExists && sdkOk + + console.log(chalk.bold('\nActive sessions')) + if (bridges.length === 0) { + console.log(chalk.yellow(' No active sessions on hub.')) + } else { + for (const b of bridges) { + const bridgeMark = b.hapiMcpUrl ? chalk.green('bridge') : chalk.yellow('no bridge') + const title = b.name ?? b.path ?? b.id + console.log( + ` ${chalk.blue(b.prefix)} ${bridgeMark} ${chalk.gray(title)}` + + (b.flavor ? chalk.gray(` (${b.flavor})`) : '') + ) + if (b.hapiMcpUrl) { + console.log(chalk.gray(` mcp: ${b.hapiMcpUrl}`)) + if (shellFallbackAvailable) { + console.log(chalk.gray(` ${formatInlineMediaCommand(scriptPath, b.prefix)}`)) + } + } + } + } + + if (listOmitsMcp) { + console.log(chalk.yellow( + '\n⚠ Some active sessions have hapiMcpUrl on detail GET but not on list — upgrade hub or use per-session GET.' + )) + } + + const cursorSessions = withBridge.filter((b) => b.flavor === 'cursor') + if (cursorSessions.length > 0) { + console.log(chalk.bold('\nCursor ACP')) + console.log(chalk.gray(' Cursor ignores session/new mcpServers. Remote sessions use ~/.cursor/mcp.json + `agent mcp enable hapi-`.')) + console.log(chalk.gray(' Tool names are bare: display_image, display_video, display_media, change_title (not hapi_display_image).')) + for (const session of cursorSessions) { + const serverId = cursorHapiMcpServerId(session.id) + console.log(chalk.gray(` Verify (${session.prefix}): agent mcp list-tools ${serverId}`)) + } + } + + console.log(chalk.bold('\nAgent inline path')) + console.log(chalk.gray(' 1. MCP tool display_image / display_video / display_media in the running session (ACP flavors via hapi bridge)')) + if (shellFallbackAvailable) { + console.log(chalk.gray(' 2. Shell fallback (HAPI session id prefix, not cursorSessionId):')) + if (withBridge.length > 0) { + console.log(chalk.green(` ${formatInlineMediaCommand(scriptPath, withBridge[0].prefix)}`)) + } else if (envSessionId) { + console.log(chalk.green(` ${formatInlineMediaCommand(scriptPath, envSessionId.slice(0, 8))}`)) + } else { + console.log(chalk.gray(` ${formatInlineMediaCommand(scriptPath, '')}`)) + } + } else { + console.log(chalk.gray(' 2. Shell fallback unavailable (packaged install / no repo checkout) — use MCP tools only')) + } + + // Core health: hub auth + live bridge or session id. Repo shell helper is optional. + const ok = jwt !== null && (withBridge.length > 0 || Boolean(envSessionId)) + + if (ok) { + console.log(chalk.green('\n✓ Inline media path available\n')) + return 0 + } + + if (withBridge.length === 0 && !envSessionId) { + console.log(chalk.yellow('\n⚠ No active session with hapiMcpUrl — start or resume a remote session first.\n')) + } else { + console.log(chalk.red('\n✗ Inline media checks failed — fix items marked ✗ above.\n')) + } + return 1 +} diff --git a/hub/src/web/routes/git.test.ts b/hub/src/web/routes/git.test.ts index 2de76f4b..5a45c607 100644 --- a/hub/src/web/routes/git.test.ts +++ b/hub/src/web/routes/git.test.ts @@ -58,6 +58,29 @@ describe('generated images route', () => { // The whole point: a cache hit must not touch the CLI over the socket. expect(rpcCalls).toBe(0) }) + + it('serves audio inline and generic files as downloads with nosniff', async () => { + const session = { id: 'session-1', namespace: 'default', active: true } as unknown as Session + let mimeType = 'audio/wav' + const engine = { + resolveSessionAccess: () => ({ ok: true as const, sessionId: 'session-1', session }), + readGeneratedImage: async () => ({ + success: true, + content: Buffer.from('media').toString('base64'), + mimeType, + fileName: mimeType === 'audio/wav' ? 'sample.wav' : 'archive.bin' + }) + } as unknown as Partial + + const audio = await buildApp(engine).request('/api/sessions/session-1/generated-images/audio-1') + expect(audio.headers.get('content-disposition')).toStartWith('inline;') + expect(audio.headers.get('x-content-type-options')).toBe('nosniff') + + mimeType = 'application/octet-stream' + const file = await buildApp(engine).request('/api/sessions/session-1/generated-images/file-1') + expect(file.headers.get('content-disposition')).toStartWith('attachment;') + expect(file.headers.get('content-type')).toContain('application/octet-stream') + }) }) describe('file search route', () => { diff --git a/hub/src/web/routes/git.ts b/hub/src/web/routes/git.ts index 5665c8eb..771f9385 100644 --- a/hub/src/web/routes/git.ts +++ b/hub/src/web/routes/git.ts @@ -190,12 +190,17 @@ export function createGitRoutes(getSyncEngine: () => SyncEngine | null): Hono socket.io RPC -> base64 round-trip every time (issue #927). return c.body(bytes, 200, { - 'Content-Type': result.mimeType ?? 'application/octet-stream', - 'Content-Disposition': `inline; filename="${encodeURIComponent(result.fileName ?? 'generated-image')}"`, + 'Content-Type': mimeType, + 'Content-Disposition': `${disposition}; filename="${encodeURIComponent(result.fileName ?? 'generated-media')}"`, + 'X-Content-Type-Options': 'nosniff', 'Cache-Control': GENERATED_IMAGE_CACHE_CONTROL, ETag: etag }) diff --git a/package.json b/package.json index 3002ad07..0b1bfc5f 100644 --- a/package.json +++ b/package.json @@ -39,9 +39,9 @@ "release-all": "cd cli && bun run release-all" }, "devDependencies": { - "@playwright/test": "^1.60.0", + "@playwright/test": "^1.61.0", "concurrently": "^9.2.1", - "playwright": "1.60.0", + "playwright": "1.61.0", "react-devtools-core": "^7.0.1", "vite-plugin-pwa": "^1.2.0" } diff --git a/playwright.config.ts b/playwright.config.ts index f24807f9..0b4b8bb7 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -5,7 +5,7 @@ const BASE_URL = `http://localhost:${PORT}` export default defineConfig({ testDir: './e2e', - // Ignore fork-local peer-stack specs if present (HAPI_PEER_*); not shipped upstream. + // Peer-stack specs need HAPI_PEER_* + mirror tooling; run via playwright.peer.config.ts only. testIgnore: ['**/peer/**'], timeout: 30_000, expect: { timeout: 5_000 }, diff --git a/scripts/dev/playwright-annotated-video.mjs b/scripts/dev/playwright-annotated-video.mjs new file mode 100644 index 00000000..f75fc354 --- /dev/null +++ b/scripts/dev/playwright-annotated-video.mjs @@ -0,0 +1,71 @@ +/** + * Playwright screencast helpers — click highlights + animated pointer on recorded video. + * + * Requires Playwright >= 1.59 (screencast.showActions); cursor animation needs >= 1.61. + * + * @playwright/test fixtures: + * import { annotatedVideoUseOption } from './scripts/dev/playwright-annotated-video.mjs' + * use: { video: process.env.PLAYWRIGHT_RECORD_VIDEO === '1' ? annotatedVideoUseOption('on') : 'off' } + * + * Programmatic (handoff .mjs scripts): + * import { startAnnotatedScreencast, stopAnnotatedScreencast } from './playwright-annotated-video.mjs' + * await startAnnotatedScreencast(page, { path: 'localdocs/playwright-runs/demo.webm' }) + * // ... interactions ... + * await stopAnnotatedScreencast(page) + */ + +/** Default overlays: element outline, action title, pointer glide between clicks. */ +export const ANNOTATED_SHOW_ACTIONS = { + position: 'top-right', + cursor: 'pointer', + duration: 800, + fontSize: 22, +} + +/** + * `use.video` value for @playwright/test when recording with action annotations. + * @param {import('@playwright/test').VideoMode} mode + * @param {import('@playwright/test').ViewportSize | undefined} size + */ +export function annotatedVideoUseOption(mode = 'on', size) { + const option = { + mode, + show: { + actions: { + position: ANNOTATED_SHOW_ACTIONS.position, + cursor: ANNOTATED_SHOW_ACTIONS.cursor, + duration: ANNOTATED_SHOW_ACTIONS.duration, + fontSize: ANNOTATED_SHOW_ACTIONS.fontSize, + }, + }, + } + if (size) option.size = size + return option +} + +export function shouldRecordAnnotatedVideo() { + return process.env.HAPI_PEER_RECORD_VIDEO === '1' || process.env.PLAYWRIGHT_RECORD_VIDEO === '1' +} + +/** + * Start annotated screencast on a page (replaces raw `recordVideo` on browser context). + * @param {import('playwright').Page} page + * @param {{ path: string, showActions?: typeof ANNOTATED_SHOW_ACTIONS, size?: { width: number, height: number } }} options + */ +export async function startAnnotatedScreencast(page, options) { + const { path, showActions = ANNOTATED_SHOW_ACTIONS, size } = options + await page.screencast.start({ path, size }) + await page.screencast.showActions(showActions) +} + +/** Stop screencast and finalize the file written by {@link startAnnotatedScreencast}. */ +export async function stopAnnotatedScreencast(page) { + await page.screencast.stop() +} + +/** Resolve webm/mp4 paths under a handoff output directory. */ +export function annotatedVideoPaths(dir, basename) { + const webm = `${dir.replace(/\/$/, '')}/${basename}.webm` + const mp4 = `${dir.replace(/\/$/, '')}/${basename}.mp4` + return { webm, mp4 } +} diff --git a/scripts/tooling/hapi-display-image.mjs b/scripts/tooling/hapi-display-image.mjs index 5a7681e6..90f51d80 100644 --- a/scripts/tooling/hapi-display-image.mjs +++ b/scripts/tooling/hapi-display-image.mjs @@ -1,23 +1,24 @@ #!/usr/bin/env bun /** - * Post a local image inline to a HAPI session via the session CLI's display_image MCP tool. + * Post a local file to a HAPI session via display_image / display_video / display_media MCP. * * Uses session.metadata.hapiMcpUrl (published at MCP server start) so we hit the MCP * endpoint, not the session hook server on another loopback port in the same process. * * Usage: * # inside a wrapped session (self-targets via $HAPI_SESSION_ID — no list): - * bun scripts/tooling/hapi-display-image.mjs [title] + * bun scripts/tooling/hapi-display-image.mjs [title] * # explicit self: - * bun scripts/tooling/hapi-display-image.mjs self [title] + * bun scripts/tooling/hapi-display-image.mjs self [title] * # explicit other session: - * bun scripts/tooling/hapi-display-image.mjs [title] + * bun scripts/tooling/hapi-display-image.mjs [title] * * Self-resolution (tiann/hapi#1119): $HAPI_SESSION_ID → GET /api/sessions/:id directly. - * Prefer the MCP display_image tool when available; this script is the shell fallback. + * Picks the strict image/video tool when recognized, else display_media. + * Prefer the MCP tools when available; this script is the shell fallback. */ -import { readFileSync, lstatSync } from 'node:fs' +import { closeSync, openSync, readSync, readFileSync, lstatSync } from 'node:fs' import { Client } from '@modelcontextprotocol/sdk/client/index.js' import { StreamableHTTPClientTransport } from '@modelcontextprotocol/sdk/client/streamableHttp.js' @@ -34,10 +35,68 @@ function isFile(p) { } } +function sessionMatchesPrefix(session, prefix) { + if (typeof session.id === 'string' && session.id.startsWith(prefix)) { + return true + } + const meta = session.metadata ?? {} + const agentIds = [ + meta.agentSessionId, + meta.cursorSessionId, + meta.codexSessionId, + meta.claudeSessionId, + meta.geminiSessionId, + meta.opencodeSessionId, + meta.kimiSessionId, + ] + return agentIds.some((id) => typeof id === 'string' && id.startsWith(prefix)) +} + +function readHeader(path, length = 16) { + const fd = openSync(path, 'r') + try { + const head = Buffer.alloc(length) + const bytesRead = readSync(fd, head, 0, head.length, 0) + return head.subarray(0, bytesRead) + } finally { + closeSync(fd) + } +} + +function detectMediaTool(path) { + // EBML DocType can sit well past the first 16 bytes; match generatedImages scan window. + const head = readHeader(path, 128) + if (head.length >= 12 && head.subarray(4, 8).toString('ascii') === 'ftyp') { + const brand = head.subarray(8, 12).toString('ascii') + if (brand === 'avif' || brand === 'avis') return 'display_image' + return 'display_media' + } + if (head.length >= 4 && head[0] === 0x1a && head[1] === 0x45 && head[2] === 0xdf && head[3] === 0xa3) { + // EBML is shared by WebM and Matroska — only route DocType "webm" to video. + const limit = Math.min(head.length, 128) + for (let i = 4; i + 3 < limit; i += 1) { + if (head[i] !== 0x42 || head[i + 1] !== 0x82) continue + const sizeByte = head[i + 2] + if ((sizeByte & 0x80) === 0) continue + const len = sizeByte & 0x7f + if (len === 0 || i + 3 + len > limit) continue + const docType = head.subarray(i + 3, i + 3 + len).toString('ascii') + if (docType === 'webm') return 'display_video' + break + } + return 'display_media' + } + if (head.length >= 8 && head.subarray(0, 8).equals(Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]))) return 'display_image' + if (head.length >= 3 && head[0] === 0xff && head[1] === 0xd8 && head[2] === 0xff) return 'display_image' + if (head.length >= 6 && ['GIF87a', 'GIF89a'].includes(head.subarray(0, 6).toString('ascii'))) return 'display_image' + if (head.length >= 12 && head.subarray(0, 4).toString('ascii') === 'RIFF' && head.subarray(8, 12).toString('ascii') === 'WEBP') return 'display_image' + return 'display_media' +} + // Arg shapes (backward compatible): -// [title] → self-target current session -// [title] → self-target, explicit -// [title] → explicit session +// [title] → self-target current session +// [title] → self-target, explicit +// [title] → explicit session const args = process.argv.slice(2) let sessionArg let imagePath @@ -53,8 +112,8 @@ if (args.length > 0 && isFile(args[0]) && !SELF_TOKENS.has(args[0])) { } if (!imagePath) { - console.error('usage: hapi-display-image.mjs [|self] [title]') - console.error(' or: HAPI_SESSION_ID= hapi-display-image.mjs [title]') + console.error('usage: hapi-display-image.mjs [|self] [title]') + console.error(' or: HAPI_SESSION_ID= hapi-display-image.mjs [title]') process.exit(2) } @@ -118,18 +177,24 @@ if (wantsSelf) { process.exit(4) } } else { - // Explicit id/prefix: full uuid → direct GET; otherwise list + prefix match. + // Explicit id/prefix: full uuid → direct GET; otherwise list + prefix match + // (HAPI id or agent session ids such as cursorSessionId). const looksFull = /^[0-9a-f-]{36}$/i.test(sessionArg) if (looksFull) { session = await fetchSessionDetail(sessionArg) } if (!session) { const sessions = await listSessions() - const listed = sessions.find((s) => typeof s.id === 'string' && s.id.startsWith(sessionArg)) - if (!listed) { - console.error(`no session for prefix ${sessionArg}`) + const matches = sessions.filter((candidate) => sessionMatchesPrefix(candidate, sessionArg)) + if (matches.length !== 1) { + console.error( + matches.length === 0 + ? `no session for prefix ${sessionArg} (use HAPI session id from /sessions/, not cursorSessionId alone)` + : `ambiguous session prefix ${sessionArg} (${matches.length} matches); use a full HAPI session id`, + ) process.exit(4) } + const listed = matches[0] // List summaries may omit hapiMcpUrl; detail fetch always has it when present. session = await fetchSessionDetail(listed.id) ?? listed } @@ -143,11 +208,12 @@ if (!mcpUrl) { console.error(`hapi-display-image: session=${session.id} mcp=${mcpUrl}`) +const mediaTool = detectMediaTool(imagePath) const client = new Client({ name: 'hapi-display-image', version: '1.0.0' }, { capabilities: {} }) const transport = new StreamableHTTPClientTransport(new URL(mcpUrl)) await client.connect(transport) const result = await client.callTool({ - name: 'display_image', + name: mediaTool, arguments: { path: imagePath, title: title ?? undefined }, }) await client.close() diff --git a/shared/src/sessionSummary.test.ts b/shared/src/sessionSummary.test.ts index a87544a4..82df97f3 100644 --- a/shared/src/sessionSummary.test.ts +++ b/shared/src/sessionSummary.test.ts @@ -166,6 +166,18 @@ describe('toSessionSummary', () => { expect(summary.metadata?.lifecycleState).toBe('archived') }) + it('includes hapiMcpUrl in summary metadata when session bridge is live', () => { + const summary = toSessionSummary(makeSession({ + metadata: { + path: '/proj', + host: 'local', + hapiMcpUrl: 'http://127.0.0.1:42133/' + } + })) + + expect(summary.metadata?.hapiMcpUrl).toBe('http://127.0.0.1:42133/') + }) + it('includes structured pendingRequests for hover-tooltip copy', () => { const summary = toSessionSummary(makeSession({ updatedAt: 5000, diff --git a/shared/src/sessionSummary.ts b/shared/src/sessionSummary.ts index 63ade7d5..0fde9b00 100644 --- a/shared/src/sessionSummary.ts +++ b/shared/src/sessionSummary.ts @@ -41,6 +41,8 @@ export type SessionSummaryMetadata = { worktree?: WorktreeMetadata agentSessionId?: string lifecycleState?: string + /** Loopback MCP URL when session CLI happy server is running (#956). */ + hapiMcpUrl?: string } export type SessionSummary = { @@ -192,7 +194,8 @@ export function toSessionSummaryMetadata(metadata: Metadata | null | undefined): flavor: metadata.flavor ?? null, worktree: metadata.worktree, agentSessionId: getSummaryAgentSessionId(metadata), - lifecycleState: metadata.lifecycleState + lifecycleState: metadata.lifecycleState, + hapiMcpUrl: metadata.hapiMcpUrl ?? undefined } } diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 79bd6fb2..ba8ed1a4 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -430,9 +430,13 @@ export class ApiClient { if (authToken) { headers.set('authorization', `Bearer ${authToken}`) } - const res = await fetch(this.buildUrl(`/api/sessions/${encodeURIComponent(sessionId)}/generated-images/${encodeURIComponent(imageId)}`), { - headers - }) + const url = this.buildUrl(`/api/sessions/${encodeURIComponent(sessionId)}/generated-images/${encodeURIComponent(imageId)}`) + let res = await fetch(url, { headers }) + // Hub returns ETag + immutable Cache-Control (#927). Default fetch cache stores 200 + // responses so remounts avoid RPC; on 304 the body is empty — read from cache. + if (res.status === 304) { + res = await fetch(url, { headers, cache: 'force-cache' }) + } if (res.status === 401 && attempt === 0 && this.onUnauthorized) { const refreshed = await this.onUnauthorized() if (refreshed) { diff --git a/web/src/chat/inlineMediaSource.ts b/web/src/chat/inlineMediaSource.ts new file mode 100644 index 00000000..956c6e89 --- /dev/null +++ b/web/src/chat/inlineMediaSource.ts @@ -0,0 +1,38 @@ +/** v1 inline media provenance (wire + chat blocks). See cli/src/modules/common/inlineMediaSource.ts */ +export type InlineMediaIngress = 'mcp' | 'acp' | 'tool_result' + +export type InlineMediaSource = { + ingress: InlineMediaIngress + flavor?: string + toolCallId?: string + toolName?: string +} + +export function inlineMediaSourceFromWire(value: unknown): InlineMediaSource | undefined { + if (!value || typeof value !== 'object' || Array.isArray(value)) return undefined + const record = value as Record + const ingress = record.ingress ?? record.path + if (ingress !== 'mcp' && ingress !== 'acp' && ingress !== 'tool_result') return undefined + const flavor = typeof record.flavor === 'string' ? record.flavor : undefined + const toolCallId = typeof record.toolCallId === 'string' + ? record.toolCallId + : typeof record.tool_call_id === 'string' + ? record.tool_call_id + : undefined + const toolName = typeof record.toolName === 'string' + ? record.toolName + : typeof record.tool_name === 'string' + ? record.tool_name + : undefined + return { ingress, flavor, toolCallId, toolName } +} + +/** Structural equality — wire normalization always allocates a fresh object. */ +export function areInlineMediaSourcesEqual(left?: InlineMediaSource, right?: InlineMediaSource): boolean { + if (left === right) return true + if (!left || !right) return false + return left.ingress === right.ingress + && left.flavor === right.flavor + && left.toolCallId === right.toolCallId + && left.toolName === right.toolName +} diff --git a/web/src/chat/normalizeAgent.ts b/web/src/chat/normalizeAgent.ts index 37f84398..d65afba4 100644 --- a/web/src/chat/normalizeAgent.ts +++ b/web/src/chat/normalizeAgent.ts @@ -1,4 +1,5 @@ import type { AgentEvent, CodexReview, CodexReviewFinding, NormalizedAgentContent, NormalizedMessage, ToolResultPermission } from '@/chat/types' +import { inlineMediaSourceFromWire } from '@/chat/inlineMediaSource' import { AGENT_MESSAGE_PAYLOAD_TYPE, asNumber, asString, isObject } from '@hapi/protocol' import { isClaudeChatVisibleMessage } from '@hapi/protocol/messages' import { parseAgentTimestampMs } from '@/chat/agentTimestamp' @@ -936,6 +937,7 @@ export function normalizeAgentRecord( const imageId = asString(data.imageId ?? data.image_id) if (!imageId) return null const uuid = asString(data.id) ?? messageId + const source = inlineMediaSourceFromWire(data.source) return { id: messageId, localId, @@ -948,7 +950,8 @@ export function normalizeAgentRecord( fileName: asString(data.fileName ?? data.file_name) ?? 'generated-image', mimeType: asString(data.mimeType ?? data.mime_type), uuid, - parentUUID: null + parentUUID: null, + source, }], meta } diff --git a/web/src/chat/reconcile.ts b/web/src/chat/reconcile.ts index 0b48e4f9..0869c379 100644 --- a/web/src/chat/reconcile.ts +++ b/web/src/chat/reconcile.ts @@ -11,6 +11,7 @@ import type { ToolPermission, UserTextBlock, } from '@/chat/types' +import { areInlineMediaSourcesEqual } from '@/chat/inlineMediaSource' export type ChatBlocksById = Map @@ -143,6 +144,7 @@ function areGeneratedImageBlocksEqual(left: GeneratedImageBlock, right: Generate && left.imageId === right.imageId && left.fileName === right.fileName && left.mimeType === right.mimeType + && areInlineMediaSourcesEqual(left.source, right.source) && left.meta === right.meta } diff --git a/web/src/chat/reducerTimeline.ts b/web/src/chat/reducerTimeline.ts index 25816f89..9cb12a9c 100644 --- a/web/src/chat/reducerTimeline.ts +++ b/web/src/chat/reducerTimeline.ts @@ -808,6 +808,7 @@ export function reduceTimeline( imageId: c.imageId, fileName: c.fileName, mimeType: c.mimeType, + source: c.source, meta: msg.meta }) continue diff --git a/web/src/chat/types.ts b/web/src/chat/types.ts index d7ff10b1..08cee34f 100644 --- a/web/src/chat/types.ts +++ b/web/src/chat/types.ts @@ -1,5 +1,6 @@ import type { AttachmentMetadata, MessageStatus } from '@/types/api' import type { ThreadGoal } from '@/types/api' +import type { InlineMediaSource } from '@/chat/inlineMediaSource' export type UsageData = { input_tokens: number @@ -70,6 +71,7 @@ export type GeneratedImageContent = { mimeType: string | null uuid: string parentUUID: string | null + source?: InlineMediaSource } export type CodexReviewFinding = { @@ -267,6 +269,7 @@ export type GeneratedImageBlock = { imageId: string fileName: string mimeType: string | null + source?: InlineMediaSource meta?: unknown } diff --git a/web/src/components/AssistantChat/messages/ToolMessage.generatedMedia.test.tsx b/web/src/components/AssistantChat/messages/ToolMessage.generatedMedia.test.tsx new file mode 100644 index 00000000..4703f864 --- /dev/null +++ b/web/src/components/AssistantChat/messages/ToolMessage.generatedMedia.test.tsx @@ -0,0 +1,92 @@ +import { describe, expect, it, vi } from 'vitest' +import { fireEvent, render, screen, waitFor } from '@testing-library/react' +import { HappyChatProvider } from '@/components/AssistantChat/context' +import { GeneratedImageCard } from '@/components/AssistantChat/messages/ToolMessage' +import type { ApiClient } from '@/api/client' +import type { HappyChatContextValue } from '@/components/AssistantChat/context' + +function renderCard(options: { + mimeType: string | null + getGeneratedImageBlob?: ReturnType +}) { + const getGeneratedImageBlob = options.getGeneratedImageBlob ?? vi.fn(async () => new Blob(['x'], { type: options.mimeType ?? 'image/png' })) + const api = { getGeneratedImageBlob } as unknown as ApiClient + const value: HappyChatContextValue = { + api, + sessionId: 'session-1', + metadata: null, + terminalToolDisplayMode: 'compact', + disabled: false, + onRefresh: () => {}, + hasMoreMessages: false, + isSyncingTail: false, + isLoadingMoreMessages: false, + loadOlderMessagesPreservingScroll: async () => 'loaded', + } + + render( + + + + ) + + return { getGeneratedImageBlob } +} + +describe('GeneratedImageCard video fetch', () => { + it('does not call the API for an untouched video card', async () => { + const { getGeneratedImageBlob } = renderCard({ mimeType: 'video/mp4' }) + + expect(screen.getByRole('button', { name: 'Load video' })).toBeInTheDocument() + await new Promise((resolve) => setTimeout(resolve, 20)) + expect(getGeneratedImageBlob).not.toHaveBeenCalled() + }) + + it('fetches the blob after the user clicks Load video', async () => { + const { getGeneratedImageBlob } = renderCard({ mimeType: 'video/mp4' }) + + fireEvent.click(screen.getByRole('button', { name: 'Load video' })) + + await waitFor(() => { + expect(getGeneratedImageBlob).toHaveBeenCalledWith('session-1', 'img-1') + }) + }) + + it('still fetches images on mount', async () => { + const { getGeneratedImageBlob } = renderCard({ mimeType: 'image/png' }) + + await waitFor(() => { + expect(getGeneratedImageBlob).toHaveBeenCalledWith('session-1', 'img-1') + }) + }) + + it('loads audio on demand and renders controls', async () => { + renderCard({ mimeType: 'audio/wav' }) + + fireEvent.click(screen.getByRole('button', { name: 'Load audio' })) + + await waitFor(() => { + expect(document.querySelector('audio[controls]')).toBeInTheDocument() + }) + }) + + it('loads unknown files on demand and renders a download link', async () => { + renderCard({ mimeType: 'application/octet-stream' }) + + fireEvent.click(screen.getByRole('button', { name: 'Prepare download' })) + + await waitFor(() => { + expect(screen.getByRole('link', { name: /Download clip\.mp4/ })).toHaveAttribute('download', 'clip.mp4') + }) + }) +}) diff --git a/web/src/components/AssistantChat/messages/ToolMessage.test.ts b/web/src/components/AssistantChat/messages/ToolMessage.test.ts new file mode 100644 index 00000000..84455aea --- /dev/null +++ b/web/src/components/AssistantChat/messages/ToolMessage.test.ts @@ -0,0 +1,24 @@ +import { describe, expect, it } from 'vitest' +import { computeTinyImageScale } from './ToolMessage' + +describe('computeTinyImageScale', () => { + it('leaves already-readable images alone', () => { + expect(computeTinyImageScale(64, 64)).toBe(1) + expect(computeTinyImageScale(128, 96)).toBe(1) + }) + + it('scales small icons up using the larger side', () => { + expect(computeTinyImageScale(16, 32)).toBe(2) + expect(computeTinyImageScale(32, 16)).toBe(2) + }) + + it('does not explode skinny images into huge transforms', () => { + expect(computeTinyImageScale(1, 1000)).toBe(1) + expect(computeTinyImageScale(1000, 1)).toBe(1) + }) + + it('rejects non-positive dimensions', () => { + expect(computeTinyImageScale(0, 32)).toBe(1) + expect(computeTinyImageScale(32, -1)).toBe(1) + }) +}) diff --git a/web/src/components/AssistantChat/messages/ToolMessage.tsx b/web/src/components/AssistantChat/messages/ToolMessage.tsx index 3aa8e443..22f5acdb 100644 --- a/web/src/components/AssistantChat/messages/ToolMessage.tsx +++ b/web/src/components/AssistantChat/messages/ToolMessage.tsx @@ -1,4 +1,4 @@ -import { useEffect, useState } from 'react' +import { useEffect, useRef, useState, type CSSProperties } from 'react' import type { ToolCallMessagePartProps } from '@assistant-ui/react' import type { ChatBlock } from '@/chat/types' import type { GeneratedImageBlock, ToolCallBlock } from '@/chat/types' @@ -15,6 +15,8 @@ import { useHappyChatContext } from '@/components/AssistantChat/context' import { CliOutputBlock } from '@/components/CliOutputBlock' import { UserBubbleContent, getUserBubbleClassName, shouldShowMessageStatus } from '@/components/AssistantChat/messages/user-bubble' import { ImagePreview } from '@/components/ImagePreview' +import { FileIcon } from '@/components/FileIcon' +import { generatedInlineMediaLabel, isInlineAudioMimeType, isInlineImageMimeType, isInlineVideoMimeType } from '@/lib/generatedInlineMedia' function isToolCallBlock(value: unknown): value is ToolCallBlock { if (!isObject(value)) return false @@ -49,53 +51,142 @@ function isGeneratedImageBlock(value: unknown): value is GeneratedImageBlock { return true } -function GeneratedImageCard(props: { block: GeneratedImageBlock }) { +const MIN_INLINE_IMAGE_DIMENSION = 64 + +/** Scale tiny icons up for readability without exploding skinny/tall images. */ +export function computeTinyImageScale(width: number, height: number): number { + const maxDim = Math.max(width, height) + if (width <= 0 || height <= 0 || maxDim >= MIN_INLINE_IMAGE_DIMENSION) { + return 1 + } + return Math.min(MIN_INLINE_IMAGE_DIMENSION / maxDim, 16) +} + +/** Exported for generated-media fetch and renderer tests. */ +export function GeneratedImageCard(props: { block: GeneratedImageBlock }) { const ctx = useHappyChatContext() const [objectUrl, setObjectUrl] = useState(null) const [error, setError] = useState(null) + const [imageStyle, setImageStyle] = useState(undefined) + const [loadMedia, setLoadMedia] = useState(false) + const objectUrlRef = useRef(null) + const isVideo = isInlineVideoMimeType(props.block.mimeType) + const isAudio = isInlineAudioMimeType(props.block.mimeType) + const isImage = isInlineImageMimeType(props.block.mimeType) + const isFile = !isVideo && !isAudio && !isImage + const mediaLabel = generatedInlineMediaLabel(props.block.mimeType) + // Non-image media can be tens of MB; wait for explicit user intent before downloading. + const shouldFetch = isImage || loadMedia useEffect(() => { - let disposed = false - let nextObjectUrl: string | null = null + return () => { + if (objectUrlRef.current) { + URL.revokeObjectURL(objectUrlRef.current) + objectUrlRef.current = null + } + } + }, []) + useEffect(() => { + if (!shouldFetch) { + return + } + + let disposed = false + + if (objectUrlRef.current) { + URL.revokeObjectURL(objectUrlRef.current) + objectUrlRef.current = null + } setObjectUrl(null) + setImageStyle(undefined) setError(null) + void ctx.api.getGeneratedImageBlob(ctx.sessionId, props.block.imageId) .then((blob) => { if (disposed) return - nextObjectUrl = URL.createObjectURL(blob) + const nextObjectUrl = URL.createObjectURL(blob) + if (objectUrlRef.current) { + URL.revokeObjectURL(objectUrlRef.current) + } + objectUrlRef.current = nextObjectUrl setObjectUrl(nextObjectUrl) + if (isImage) { + setImageStyle(undefined) + const probe = new Image() + probe.onload = () => { + if (disposed) return + const scale = computeTinyImageScale(probe.naturalWidth, probe.naturalHeight) + setImageStyle(scale === 1 ? undefined : { transform: `scale(${scale})` }) + } + probe.src = nextObjectUrl + } }) .catch((err: unknown) => { if (disposed) return - setError(err instanceof Error ? err.message : 'Failed to load generated image') + setError(err instanceof Error ? err.message : 'Failed to load inline media') }) return () => { disposed = true - if (nextObjectUrl) { - URL.revokeObjectURL(nextObjectUrl) - } } - }, [ctx.api, ctx.sessionId, props.block.imageId]) + }, [ctx.api, ctx.sessionId, props.block.imageId, isImage, shouldFetch]) return (
- Generated image · {props.block.fileName} + {mediaLabel} · {props.block.fileName}
{objectUrl ? ( - + isVideo ? ( +
+
+ ) : isAudio ? ( +