diff --git a/.changeset/fix-skill-command-media.md b/.changeset/fix-skill-command-media.md new file mode 100644 index 0000000000..60e088b137 --- /dev/null +++ b/.changeset/fix-skill-command-media.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix pasted media being dropped from /skill and plugin command arguments. diff --git a/.changeset/fix-steer-media.md b/.changeset/fix-steer-media.md new file mode 100644 index 0000000000..5d0f8ac61e --- /dev/null +++ b/.changeset/fix-steer-media.md @@ -0,0 +1,5 @@ +--- +"@moonshot-ai/kimi-code": patch +--- + +Fix pasted images being dropped when steering with Ctrl-S. diff --git a/apps/kimi-code/src/tui/controllers/editor-keyboard.ts b/apps/kimi-code/src/tui/controllers/editor-keyboard.ts index 383694c073..5b6d95e195 100644 --- a/apps/kimi-code/src/tui/controllers/editor-keyboard.ts +++ b/apps/kimi-code/src/tui/controllers/editor-keyboard.ts @@ -15,7 +15,8 @@ import { } from '../constant/kimi-tui'; import { formatErrorMessage } from '../utils/event-payload'; import type { ImageAttachmentStore } from '../utils/image-attachment-store'; -import type { PendingExit, QueuedMessage } from '../types'; +import { extractMediaAttachments } from '../utils/image-placeholder'; +import type { PendingExit, QueuedMessage, SteerInputItem } from '../types'; import type { TUIState } from '../tui-state'; import type { BtwPanelController } from './btw-panel'; @@ -32,7 +33,12 @@ export interface EditorKeyboardHost { handleUserInput(text: string): void; readonly btwPanelController: BtwPanelController; - steerMessage(session: Session, input: string[]): void; + steerMessage(session: Session, input: readonly SteerInputItem[]): void; + validateMediaCapabilities(extraction: { + hasMedia: boolean; + imageAttachmentIds: readonly number[]; + videoAttachmentIds: readonly number[]; + }): boolean; recallLastQueued(): QueuedMessage | undefined; showError(msg: string): void; track(event: string, props?: Record): void; @@ -250,22 +256,53 @@ export class EditorKeyboardController { // after the current task instead of being injected into the turn as text. const queued = host.state.queuedMessages; const steerable = queued.filter((m) => m.mode !== 'bash'); - host.state.queuedMessages = queued.filter((m) => m.mode === 'bash'); - const parts: string[] = []; + const items: SteerInputItem[] = []; for (const m of steerable) { const trimmed = m.text.trim(); - if (trimmed.length > 0) parts.push(trimmed); + if (trimmed.length > 0) { + // Queued items carry the parts extracted when they were submitted + // (and were already capability-validated then). + items.push({ text: trimmed, parts: m.parts, imageAttachmentIds: m.imageAttachmentIds }); + } + } + let editorExtraction: ReturnType | undefined; + if (!editorIsBash && text.length > 0) { + try { + editorExtraction = extractMediaAttachments(text, this.imageStore); + } catch (error) { + // Cache copy failed (e.g. the pasted video's source vanished) — + // leave the queue and the editor draft untouched. + host.showError(`Failed to prepare media attachment: ${formatErrorMessage(error)}`); + return; + } + items.push({ + text, + parts: editorExtraction.hasMedia ? editorExtraction.parts : undefined, + imageAttachmentIds: + editorExtraction.imageAttachmentIds.length > 0 + ? editorExtraction.imageAttachmentIds + : undefined, + }); } - if (!editorIsBash && text.length > 0) parts.push(text); - if (parts.length > 0) { + if (items.length > 0) { + // The editor draft is fresh input: gate it on the model's media + // capabilities before splicing the queue, so a rejection leaves the + // queue and the draft untouched. + if ( + editorExtraction !== undefined && + !host.validateMediaCapabilities(editorExtraction) + ) { + return; + } + host.state.queuedMessages = queued.filter((m) => m.mode === 'bash'); if (!editorIsBash) editor.setText(''); const session = host.session; if (host.state.appState.model.trim().length === 0 || session === undefined) { host.showError(LLM_NOT_SET_MESSAGE); } else { - host.steerMessage(session, parts); + host.steerMessage(session, items); } } host.updateQueueDisplay(); diff --git a/apps/kimi-code/src/tui/kimi-tui.ts b/apps/kimi-code/src/tui/kimi-tui.ts index 62d1b2370b..a05165ca2b 100644 --- a/apps/kimi-code/src/tui/kimi-tui.ts +++ b/apps/kimi-code/src/tui/kimi-tui.ts @@ -123,6 +123,7 @@ import { type LivePaneState, type LoginProgressSpinnerHandle, type QueuedMessage, + type SteerInputItem, type TranscriptEntry, type TUIStartupOptions, type TUIStartupState, @@ -132,7 +133,7 @@ import { isDeadTerminalError } from './utils/dead-terminal'; import { formatErrorMessage } from './utils/event-payload'; import { pickForegroundTasks } from './utils/foreground-task'; import { ImageAttachmentStore, type ImageAttachment } from './utils/image-attachment-store'; -import { extractMediaAttachments } from './utils/image-placeholder'; +import { extractMediaAttachments, rewriteMediaPlaceholders } from './utils/image-placeholder'; import { hasPatchChanges } from './utils/object-patch'; import { sessionRowsForPicker } from './utils/session-picker-rows'; import { formatBashOutputForDisplay } from './utils/shell-output'; @@ -238,6 +239,50 @@ interface SendMessageOptions { readonly hasMedia?: boolean; } +/** + * Flatten steer items into the payload `session.steer` expects: the + * historical `'\n\n'`-joined string when nothing carries media, or a + * merged part list when any item has extracted media parts (queued image + * messages, or the editor draft after placeholder extraction). + * + * Items are separated by the historical `'\n\n'`, which merges into the + * adjacent text part. The one exception is two touching media parts: a + * standalone `{type:'text',text:'\n\n'}` between them would be rejected + * by `normalizePromptInput` as an empty text part, so the separator is + * dropped there (media parts are self-delimiting anyway). + */ +function combineSteerInput(items: readonly SteerInputItem[]): string | PromptPart[] { + const hasMedia = items.some((item) => item.parts !== undefined && item.parts.length > 0); + if (!hasMedia) return items.map((item) => item.text).join('\n\n'); + const parts: PromptPart[] = []; + for (const item of items) { + const startsWithMedia = + item.parts !== undefined && item.parts.length > 0 && item.parts[0]?.type !== 'text'; + const lastIsMedia = parts.length > 0 && parts.at(-1)?.type !== 'text'; + if (parts.length > 0 && !(lastIsMedia && startsWithMedia)) { + appendSteerText(parts, '\n\n'); + } + if (item.parts !== undefined && item.parts.length > 0) { + for (const part of item.parts) { + if (part.type === 'text') appendSteerText(parts, part.text); + else parts.push(part); + } + } else { + appendSteerText(parts, item.text); + } + } + return parts; +} + +function appendSteerText(parts: PromptPart[], text: string): void { + const last = parts.at(-1); + if (last?.type === 'text') { + parts[parts.length - 1] = { type: 'text', text: last.text + text }; + return; + } + parts.push({ type: 'text', text }); +} + /** How long the one-shot "moved to background" footer hint stays visible. */ const DETACH_HINT_DISPLAY_MS = 4_000; @@ -1095,9 +1140,11 @@ export class KimiTUI { this.state.ui.requestRender(); } - private validateMediaCapabilities( - extraction: ReturnType, - ): boolean { + validateMediaCapabilities(extraction: { + hasMedia: boolean; + imageAttachmentIds: readonly number[]; + videoAttachmentIds: readonly number[]; + }): boolean { if (!extraction.hasMedia) return true; if ( extraction.imageAttachmentIds.length > 0 && @@ -1243,8 +1290,22 @@ export class KimiTUI { } sendSkillActivation(session: Session, skillName: string, skillArgs: string): void { + // Args are a plain-text channel, so pasted media can't ride along as + // inline parts. Skill args are XML-escaped on render (renderSkillAttributes + // + expandSkillParameters), so rewrite placeholders into escape-proof + // plain-text file references the model can open with ReadMediaFile. + let rewrite: ReturnType; + try { + rewrite = rewriteMediaPlaceholders(skillArgs, this.imageStore, 'plain'); + } catch (error) { + // Cache copy failed (unwritable cache dir, vanished video source…); + // nothing has been dispatched yet, so just report and keep the input. + this.showError(`Failed to prepare media attachment: ${formatErrorMessage(error)}`); + return; + } + if (!this.validateMediaCapabilities(rewrite)) return; this.beginSessionRequest(); - void session.activateSkill(skillName, skillArgs).catch((error: unknown) => { + void session.activateSkill(skillName, rewrite.text).catch((error: unknown) => { const message = formatErrorMessage(error); this.failSessionRequest(`Skill "${skillName}" failed: ${message}`); }); @@ -1256,11 +1317,24 @@ export class KimiTUI { commandName: string, args: string, ): void { + // Plugin command args are expanded verbatim (no XML escaping), so the + // standard tag convention works — see + // sendSkillActivation for the escaped-channel variant. + let rewrite: ReturnType; + try { + rewrite = rewriteMediaPlaceholders(args, this.imageStore, 'tag'); + } catch (error) { + this.showError(`Failed to prepare media attachment: ${formatErrorMessage(error)}`); + return; + } + if (!this.validateMediaCapabilities(rewrite)) return; this.beginSessionRequest(); - void session.activatePluginCommand(pluginId, commandName, args).catch((error: unknown) => { - const message = formatErrorMessage(error); - this.failSessionRequest(`Command "${pluginId}:${commandName}" failed: ${message}`); - }); + void session + .activatePluginCommand(pluginId, commandName, rewrite.text) + .catch((error: unknown) => { + const message = formatErrorMessage(error); + this.failSessionRequest(`Command "${pluginId}:${commandName}" failed: ${message}`); + }); } private sendMessage(session: Session, input: string, options?: SendMessageOptions): void { @@ -1275,31 +1349,35 @@ export class KimiTUI { this.sendMessageInternal(session, input, options); } - steerMessage(session: Session, input: string[]): void { + steerMessage(session: Session, input: readonly SteerInputItem[]): void { if (this.deferUserMessages || this.state.appState.isCompacting) { - for (const part of input) { - this.enqueueMessage(part); + for (const item of input) { + this.enqueueMessage(item.text, item); } return; } if (this.state.appState.streamingPhase === 'idle') { - for (const part of input) { - this.sendMessageInternal(session, part); + for (const item of input) { + this.sendMessageInternal(session, item.text, item); } return; } - for (const part of input) { + for (const item of input) { this.appendTranscriptEntry({ id: nextTranscriptId(), kind: 'user', turnId: this.streamingUI.getTurnContext().turnId, renderMode: 'plain', - content: part, + content: item.text, + imageAttachmentIds: + item.imageAttachmentIds !== undefined && item.imageAttachmentIds.length > 0 + ? item.imageAttachmentIds + : undefined, }); } - void session.steer(input.join('\n\n')).catch((error: unknown) => { + void session.steer(combineSteerInput(input)).catch((error: unknown) => { const message = formatErrorMessage(error); this.showError(`Failed to steer: ${message}`); }); diff --git a/apps/kimi-code/src/tui/types.ts b/apps/kimi-code/src/tui/types.ts index 6dcdccdd1a..e79d2c8b40 100644 --- a/apps/kimi-code/src/tui/types.ts +++ b/apps/kimi-code/src/tui/types.ts @@ -206,6 +206,18 @@ export interface QueuedMessage { readonly mode?: 'prompt' | 'bash'; } +/** + * One unit of Ctrl-S steer input: a queued message or the editor draft, + * with the media parts extracted at submit/paste time so images and video + * tags survive the steer path (which accepts full prompt parts, not just + * text). + */ +export interface SteerInputItem { + readonly text: string; + readonly parts?: readonly PromptPart[]; + readonly imageAttachmentIds?: readonly number[]; +} + export const INITIAL_LIVE_PANE: LivePaneState = { mode: 'idle', pendingApproval: null, diff --git a/apps/kimi-code/src/tui/utils/image-placeholder.ts b/apps/kimi-code/src/tui/utils/image-placeholder.ts index 4017c4b2d1..56edcee88b 100644 --- a/apps/kimi-code/src/tui/utils/image-placeholder.ts +++ b/apps/kimi-code/src/tui/utils/image-placeholder.ts @@ -18,7 +18,7 @@ */ import { randomUUID } from 'node:crypto'; -import { copyFileSync, mkdirSync } from 'node:fs'; +import { copyFileSync, mkdirSync, writeFileSync } from 'node:fs'; import { join } from 'node:path'; import type { PromptPart } from '@moonshot-ai/kimi-code-sdk'; @@ -101,6 +101,78 @@ export function extractMediaAttachments( }; } +export interface MediaTagRewriteResult { + /** Input text with resolved placeholders replaced by media references. */ + text: string; + hasMedia: boolean; + imageAttachmentIds: number[]; + videoAttachmentIds: number[]; +} + +/** + * How a resolved placeholder is rendered into command args: + * - `'tag'`: the `` convention, for channels + * that pass args through verbatim (plugin commands). + * - `'plain'`: a plain-text file reference with no XML tag/attribute + * boundary characters, for channels that XML-escape args (`/skill` + * args are escaped by both `renderSkillAttributes` and + * `expandSkillParameters`, which would mangle the tag form). + */ +export type MediaReferenceStyle = 'tag' | 'plain'; + +/** + * Rewrite media placeholders in slash-command args (`/skill:foo …`, + * plugin commands) into references pointing at cache-dir copies. Command + * args are a plain-text channel — unlike `extractMediaAttachments`, which + * inlines image parts for the prompt endpoint — so the model reaches the + * media through `ReadMediaFile` instead, the same way it already handles + * pasted videos. + * + * Surrounding text is preserved verbatim (args are user content, not + * LLM parts), and unresolved placeholders stay literal. + */ +export function rewriteMediaPlaceholders( + text: string, + store: ImageAttachmentStore, + style: MediaReferenceStyle = 'tag', +): MediaTagRewriteResult { + const imageAttachmentIds: number[] = []; + const videoAttachmentIds: number[] = []; + let cursor = 0; + let out = ''; + + PLACEHOLDER_REGEX.lastIndex = 0; + let match: RegExpExecArray | null; + while ((match = PLACEHOLDER_REGEX.exec(text)) !== null) { + const [literal, kind, idStr] = match; + if (kind !== 'image' && kind !== 'video') continue; + if (idStr === undefined) continue; + const id = Number.parseInt(idStr, 10); + const attachment = store.get(id); + if (attachment === undefined) continue; // stale / user-typed — leave as text + if (attachment.kind !== kind) continue; + out += text.slice(cursor, match.index); + if (attachment.kind === 'video') { + const path = materializeVideoToCache(attachment, style === 'plain'); + out += style === 'plain' ? formatMediaReference('video', path) : formatMediaTag('video', path); + videoAttachmentIds.push(id); + } else { + const path = materializeImageToCache(attachment); + out += style === 'plain' ? formatMediaReference('image', path) : formatMediaTag('image', path); + imageAttachmentIds.push(id); + } + cursor = match.index + literal.length; + } + + const hasMedia = imageAttachmentIds.length + videoAttachmentIds.length > 0; + return { + text: hasMedia ? out + text.slice(cursor) : text, + hasMedia, + imageAttachmentIds, + videoAttachmentIds, + }; +} + function pushText(parts: PromptPart[], segment: string): void { if (segment.length === 0) return; // Keep whitespace-only segments only when they sit between non-empty @@ -123,14 +195,38 @@ function imagePartForAttachment(att: ImageAttachment): PromptPart { }; } -function materializeVideoToCache(att: VideoAttachment): string { +function materializeVideoToCache(att: VideoAttachment, escapeProofName = false): string { const cacheDir = getCacheDir(); mkdirSync(cacheDir, { recursive: true }); - const target = join(cacheDir, `${randomUUID()}-${att.label}`); + // The label permits XML boundary chars (`<>&"`); plain references go + // through skill-arg escaping, where they would no longer match the file + // on disk, so strip them from the cache name in that mode. + const label = escapeProofName ? att.label.replaceAll(/[<>&"]/g, '_') : att.label; + const target = join(cacheDir, `${randomUUID()}-${label}`); copyFileSync(att.sourcePath, target); return target; } +const IMAGE_MIME_EXTENSION: Readonly> = { + 'image/png': 'png', + 'image/jpeg': 'jpg', + 'image/gif': 'gif', + 'image/webp': 'webp', + 'image/bmp': 'bmp', + 'image/tiff': 'tif', +}; + +function materializeImageToCache(att: ImageAttachment): string { + const cacheDir = getCacheDir(); + mkdirSync(cacheDir, { recursive: true }); + // ReadMediaFile sniffs the real format from the bytes, so the extension + // only needs to be a reasonable hint. + const ext = IMAGE_MIME_EXTENSION[att.mime.trim().toLowerCase()] ?? 'img'; + const target = join(cacheDir, `${randomUUID()}.${ext}`); + writeFileSync(target, att.bytes); + return target; +} + function captionForCompressedImage(att: ImageAttachment): string { const original = att.original; if (original === undefined) return ''; @@ -155,6 +251,16 @@ function formatMediaTag(tag: 'image' | 'video', path: string): string { return `<${tag} path="${escapeAttribute(path)}">`; } +/** + * Plain-text media reference for channels that XML-escape args (`/skill`). + * Free of `& < > "` (UUID image names; boundary chars stripped from video + * cache names — see materializeVideoToCache) so it survives + * `escapeXml`/`escapeXmlTags` untouched. + */ +function formatMediaReference(kind: 'image' | 'video', path: string): string { + return `Attached ${kind} file: ${path} (open it with ReadMediaFile)`; +} + function escapeAttribute(value: string): string { return value .replaceAll('&', '&') diff --git a/apps/kimi-code/test/tui/input/image-placeholder.test.ts b/apps/kimi-code/test/tui/input/image-placeholder.test.ts index e157cbf8c8..477727293a 100644 --- a/apps/kimi-code/test/tui/input/image-placeholder.test.ts +++ b/apps/kimi-code/test/tui/input/image-placeholder.test.ts @@ -6,7 +6,10 @@ import { describe, it, expect } from 'vitest'; import { KIMI_CODE_HOME_ENV } from '#/constant/app'; import { ImageAttachmentStore } from '#/tui/utils/image-attachment-store'; -import { extractMediaAttachments } from '#/tui/utils/image-placeholder'; +import { + extractMediaAttachments, + rewriteMediaPlaceholders, +} from '#/tui/utils/image-placeholder'; import { getCacheDir } from '#/utils/paths'; function storeWith( @@ -221,3 +224,148 @@ describe('extractMediaAttachments', () => { expect(r.parts[0]?.type).toBe('image_url'); }); }); + +describe('rewriteMediaPlaceholders', () => { + it('returns plain text untouched with hasMedia=false', () => { + const store = new ImageAttachmentStore(); + const r = rewriteMediaPlaceholders('just some args', store); + expect(r.text).toBe('just some args'); + expect(r.hasMedia).toBe(false); + expect(r.imageAttachmentIds).toEqual([]); + expect(r.videoAttachmentIds).toEqual([]); + }); + + it('rewrites an image placeholder into a cache-path image tag', () => { + const { cleanup } = setupTempCache(); + try { + const bytes = new Uint8Array([0x89, 0x50, 0x4e, 0x47]); + const { store, placeholder } = storeWith(bytes); + const r = rewriteMediaPlaceholders(`look at ${placeholder} please`, store); + expect(r.hasMedia).toBe(true); + expect(r.imageAttachmentIds).toEqual([1]); + const m = /^look at <\/image> please$/.exec(r.text); + if (!m) throw new Error(`no image tag found in: ${r.text}`); + expect(m[1]!.startsWith(getCacheDir())).toBe(true); + expect(m[1]!.endsWith('.png')).toBe(true); + expect(new Uint8Array(readFileSync(m[1]!))).toEqual(bytes); + } finally { + cleanup(); + } + }); + + it('rewrites a video placeholder into a cache-path video tag', () => { + const { cleanup } = setupTempCache(); + const srcDir = makeTempDir(); + try { + const srcVideo = join(srcDir, 'clip.mov'); + writeFileSync(srcVideo, 'video-bytes'); + const store = new ImageAttachmentStore(); + const att = store.addVideo('video/quicktime', srcVideo); + const r = rewriteMediaPlaceholders(att.placeholder, store); + expect(r.hasMedia).toBe(true); + expect(r.videoAttachmentIds).toEqual([1]); + const m = /