From 93db34b0d52259f2e34ecf69a1c61dfc14a3d105 Mon Sep 17 00:00:00 2001 From: can1357 Date: Mon, 22 Jun 2026 06:11:57 +0200 Subject: [PATCH] refactor(coding-agent): consolidated editor state and unify transcript rendering - Centralized draft state and image management by migrating fields from context to the CustomEditor component. - Standardized transcript row construction by introducing shared helpers for background jobs, IRC traffic, and file mentions. - Refactored redundant UI logic and helper functions into reusable utility modules to streamline message submission and component rendering. - Standardized event handler types by consolidating lifecycle definitions into a shared module while maintaining public API stability. --- packages/coding-agent/CHANGELOG.md | 3 + packages/coding-agent/src/dap/client.ts | 2 +- .../src/extensibility/extensions/runner.ts | 15 +- .../src/extensibility/hooks/loader.ts | 24 +-- .../extensibility/session-handler-types.ts | 21 +++ packages/coding-agent/src/lsp/client.ts | 2 +- .../components/chat-transcript-builder.ts | 120 ++----------- .../src/modes/components/custom-editor.ts | 19 +++ .../src/modes/controllers/event-controller.ts | 12 +- .../src/modes/controllers/input-controller.ts | 86 ++++------ .../src/modes/interactive-mode.ts | 12 +- packages/coding-agent/src/modes/types.ts | 2 - .../utils/interactive-context-helpers.ts | 27 +++ .../modes/utils/transcript-render-helpers.ts | 157 ++++++++++++++++++ .../src/modes/utils/ui-helpers.ts | 147 +++------------- .../coding-agent/src/prompts/tools/find.md | 1 - .../input-controller-compaction-image.test.ts | 40 +++-- .../test/input-controller-escape.test.ts | 17 +- .../input-controller-followup-image.test.ts | 20 ++- .../test/input-controller-keybindings.test.ts | 23 ++- .../input-controller-orphan-submit.test.ts | 19 ++- .../input-controller-python-prefix.test.ts | 7 +- .../test/input-controller-skill-queue.test.ts | 9 +- .../test/issue-2375-repro.test.ts | 19 ++- ...interrupt-and-flush-empty-messages.test.ts | 5 +- 25 files changed, 419 insertions(+), 390 deletions(-) create mode 100644 packages/coding-agent/src/extensibility/session-handler-types.ts create mode 100644 packages/coding-agent/src/modes/utils/interactive-context-helpers.ts create mode 100644 packages/coding-agent/src/modes/utils/transcript-render-helpers.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index c4d8fb083..deaaf9d38 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,8 +1,11 @@ # Changelog ## [Unreleased] + ### Changed +- Refactored internal draft state to consolidate images and text into the editor component +- Unified transcript rendering logic for background jobs, IRC traffic, and file mentions - Unified subprocess lifecycle management for mnemopi, speech, tiny-model, and TTS workers - Softened the system-prompt and tool-prompt guidance that hard-forbade reaching for shell equivalents of dedicated tools (`grep`/`rg`, `sed`/`perl -i`, `cat`/`head`/`tail`, `find`/`fd`). The "Specialized Tool Priority" section, the `bash`/`search`/`find`/`read`/`replace` tool prompts, and the FORBIDDEN/NEVER framing now state the dedicated tools as the preferred default rather than an absolute prohibition, so the agent no longer fights itself when a quick shell command is the right call. The opt-in `bashInterceptor` (default off) still hard-blocks these commands for users who enable it. - Unified subprocess lifecycle management for mnemopi, speech, tiny-model, and TTS workers diff --git a/packages/coding-agent/src/dap/client.ts b/packages/coding-agent/src/dap/client.ts index dd3e12372..46740d80b 100644 --- a/packages/coding-agent/src/dap/client.ts +++ b/packages/coding-agent/src/dap/client.ts @@ -400,7 +400,7 @@ export class DapClient { framer.push(Buffer.from(value)); // Drain every complete message currently buffered. - for (const messageText of framer.drain((headerText) => { + for (const messageText of framer.drain(headerText => { // Non-protocol bytes (e.g. an adapter printing to stdout). // Drop past the bogus terminator and resync instead of // stalling on the same junk header forever. diff --git a/packages/coding-agent/src/extensibility/extensions/runner.ts b/packages/coding-agent/src/extensibility/extensions/runner.ts index 1889c76e6..5dea8407e 100644 --- a/packages/coding-agent/src/extensibility/extensions/runner.ts +++ b/packages/coding-agent/src/extensibility/extensions/runner.ts @@ -10,6 +10,7 @@ import type { Settings } from "../../config/settings"; import type { MemoryRuntimeContext } from "../../memory-backend"; import { type Theme, theme } from "../../modes/theme/theme"; import type { SessionManager } from "../../session/session-manager"; +import type { BranchHandler, NavigateTreeHandler, NewSessionHandler } from "../session-handler-types"; import { createExtensionModelQuery } from "./model-api"; import type { AfterProviderResponseEvent, @@ -141,17 +142,9 @@ type RunnerEmitResult = TEvent extends { type: " ? SessionStopEventResult | undefined : undefined; -export type NewSessionHandler = (options?: { - parentSession?: string; - setup?: (sessionManager: SessionManager) => Promise; -}) => Promise<{ cancelled: boolean }>; - -export type BranchHandler = (entryId: string) => Promise<{ cancelled: boolean }>; - -export type NavigateTreeHandler = ( - targetId: string, - options?: { summarize?: boolean }, -) => Promise<{ cancelled: boolean }>; +// Session-lifecycle handler types live once in session-handler-types (imported +// above for local use); re-exported here to keep this module's public API stable. +export type { BranchHandler, NavigateTreeHandler, NewSessionHandler }; export type SwitchSessionHandler = (sessionPath: string) => Promise<{ cancelled: boolean }>; diff --git a/packages/coding-agent/src/extensibility/hooks/loader.ts b/packages/coding-agent/src/extensibility/hooks/loader.ts index 343252480..fb08281a2 100644 --- a/packages/coding-agent/src/extensibility/hooks/loader.ts +++ b/packages/coding-agent/src/extensibility/hooks/loader.ts @@ -11,7 +11,6 @@ import { loadCapability } from "../../discovery"; // Runtime self-reference: dereference this namespace only inside loader functions to keep the index.ts cycle safe. import * as PiCodingAgent from "../../index"; import type { HookMessage } from "../../session/messages"; -import type { SessionManager } from "../../session/session-manager"; import * as typebox from "../typebox"; import { resolvePath } from "../utils"; import { execCommand } from "./runner"; @@ -35,26 +34,9 @@ export type SendMessageHandler = ( */ export type AppendEntryHandler = (customType: string, data?: T) => void; -/** - * New session handler type for ctx.newSession() in HookCommandContext. - */ -export type NewSessionHandler = (options?: { - parentSession?: string; - setup?: (sessionManager: SessionManager) => Promise; -}) => Promise<{ cancelled: boolean }>; - -/** - * Branch handler type for ctx.branch() in HookCommandContext. - */ -export type BranchHandler = (entryId: string) => Promise<{ cancelled: boolean }>; - -/** - * Navigate tree handler type for ctx.navigateTree() in HookCommandContext. - */ -export type NavigateTreeHandler = ( - targetId: string, - options?: { summarize?: boolean }, -) => Promise<{ cancelled: boolean }>; +// Session-lifecycle handler types live once in session-handler-types; re-exported +// here because hooks/runner.ts imports them from this module. +export type { BranchHandler, NavigateTreeHandler, NewSessionHandler } from "../session-handler-types"; /** * Registered handlers for a loaded hook. diff --git a/packages/coding-agent/src/extensibility/session-handler-types.ts b/packages/coding-agent/src/extensibility/session-handler-types.ts new file mode 100644 index 000000000..1fcc4c41b --- /dev/null +++ b/packages/coding-agent/src/extensibility/session-handler-types.ts @@ -0,0 +1,21 @@ +/** + * Session-lifecycle handler types shared by the extension runner and the hook + * loader/runner. Both surfaces wire the same new-session / branch / navigate + * handlers into their command contexts; this is the single source of truth. + */ +import type { SessionManager } from "../session/session-manager"; + +/** Handler for `ctx.newSession()` — creates (and optionally seeds) a session. */ +export type NewSessionHandler = (options?: { + parentSession?: string; + setup?: (sessionManager: SessionManager) => Promise; +}) => Promise<{ cancelled: boolean }>; + +/** Handler for `ctx.branch()` — branches from a transcript entry. */ +export type BranchHandler = (entryId: string) => Promise<{ cancelled: boolean }>; + +/** Handler for `ctx.navigateTree()` — navigates the session tree. */ +export type NavigateTreeHandler = ( + targetId: string, + options?: { summarize?: boolean }, +) => Promise<{ cancelled: boolean }>; diff --git a/packages/coding-agent/src/lsp/client.ts b/packages/coding-agent/src/lsp/client.ts index 863e6d221..343de7ab6 100644 --- a/packages/coding-agent/src/lsp/client.ts +++ b/packages/coding-agent/src/lsp/client.ts @@ -222,7 +222,7 @@ async function startMessageReader(client: LspClient): Promise { framer.push(Buffer.from(value)); // Drain every complete message currently buffered. - for (const messageText of framer.drain((headerText) => { + for (const messageText of framer.drain(headerText => { // Non-protocol bytes on stdout (e.g. a wrapper script printing). // Drop past the bogus terminator and resync instead of stalling // on the same junk header forever. diff --git a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts index 02e8a3f2a..9a854d71f 100644 --- a/packages/coding-agent/src/modes/components/chat-transcript-builder.ts +++ b/packages/coding-agent/src/modes/components/chat-transcript-builder.ts @@ -13,8 +13,7 @@ */ import type { AgentMessage, AgentTool } from "@oh-my-pi/pi-agent-core"; import type { Usage } from "@oh-my-pi/pi-ai"; -import { Text, type TUI } from "@oh-my-pi/pi-tui"; -import { formatBytes, formatDuration } from "@oh-my-pi/pi-utils"; +import type { TUI } from "@oh-my-pi/pi-tui"; import type { AdvisorMessageDetails } from "../../advisor"; import { COLLAB_PROMPT_MESSAGE_TYPE, type CollabPromptDetails } from "../../collab/protocol"; import { settings } from "../../config/settings"; @@ -22,16 +21,20 @@ import type { MessageRenderer } from "../../extensibility/extensions/types"; import { BACKGROUND_TAN_DISPATCH_MESSAGE_TYPE, type CustomMessage, - isSilentAbort, LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE, - resolveAbortLabel, SKILL_PROMPT_MESSAGE_TYPE, type SkillPromptDetails, } from "../../session/messages"; import type { SessionMessageEntry } from "../../session/session-entries"; -import { createIrcMessageCard } from "../../tools/irc"; -import { canonicalizeMessage } from "../../utils/thinking-display"; import { theme } from "../theme/theme"; +import { + assistantHasVisibleContent, + buildAsyncResultBlock, + buildFileMentionBlock, + buildIrcMessageCard, + normalizeToolArgs, + resolveAssistantErrorMessage, +} from "../utils/transcript-render-helpers"; import { createAdvisorMessageCard } from "./advisor-message"; import { AssistantMessageComponent } from "./assistant-message"; import { createBackgroundTanDispatchBlock } from "./background-tan-message"; @@ -49,7 +52,7 @@ import { type LateDiagnosticsFile, LateDiagnosticsMessageComponent } from "./lat import { ReadToolGroupComponent, readArgsHaveTarget, readArgsTargetInternalUrl } from "./read-tool-group"; import { SkillMessageComponent } from "./skill-message"; import { ToolExecutionComponent } from "./tool-execution"; -import { TranscriptBlock, TranscriptContainer } from "./transcript-container"; +import { TranscriptContainer } from "./transcript-container"; import { createUsageRowBlock } from "./usage-row"; import { UserMessageComponent } from "./user-message"; @@ -218,27 +221,9 @@ export class ChatTranscriptBuilder { break; } case "fileMention": { - const block = new TranscriptBlock(); - for (const file of message.files) { - let suffix: string; - if (file.skippedReason === "tooLarge") { - const size = typeof file.byteSize === "number" ? formatBytes(file.byteSize) : "unknown size"; - suffix = `(skipped: ${size})`; - } else { - suffix = file.image - ? "(image)" - : file.lineCount === undefined - ? "(unknown lines)" - : `(${file.lineCount} lines)`; - } - const text = `${theme.fg("dim", `${theme.tree.last} `)}${theme.fg("muted", "Read")} ${theme.fg( - "accent", - file.path, - )} ${theme.fg("dim", suffix)}`; - // Indent one column to match the transcript's other rows (the viewer renders - // body rows without an outer gutter; rows own their left pad). - block.addChild(new Text(text, 1, 0)); - } + // Indent one column to match the transcript's other rows (the viewer renders + // body rows without an outer gutter; rows own their left pad). + const block = buildFileMentionBlock(message.files, 1); if (block.children.length > 0) this.container.addChild(block); break; } @@ -266,24 +251,14 @@ export class ChatTranscriptBuilder { this.#lastAssistantUsage = message.usage; } - const hasVisibleAssistantContent = message.content.some( - content => - (content.type === "text" && canonicalizeMessage(content.text)) || - (content.type === "thinking" && canonicalizeMessage(content.thinking)), - ); + const hasVisibleAssistantContent = assistantHasVisibleContent(message); if (hasVisibleAssistantContent) { // New visible turn content closes the current read run (mirrors rebuild). this.#readGroup?.seal(); this.#readGroup = null; } - const isAbortedSilently = message.stopReason === "aborted" && isSilentAbort(message.errorMessage); - const hasErrorStop = !isAbortedSilently && (message.stopReason === "aborted" || message.stopReason === "error"); - const errorMessage = hasErrorStop - ? message.stopReason === "aborted" - ? resolveAbortLabel(message.errorMessage) - : message.errorMessage || "Error" - : null; + const { hasErrorStop, errorMessage } = resolveAssistantErrorMessage(message); for (const content of message.content) { if (content.type !== "toolCall") continue; @@ -303,10 +278,7 @@ export class ChatTranscriptBuilder { content.id, ); } else { - const normalizedArgs = - content.arguments && typeof content.arguments === "object" && !Array.isArray(content.arguments) - ? (content.arguments as Record) - : {}; + const normalizedArgs = normalizeToolArgs(content.arguments); this.#readArgs.set(content.id, normalizedArgs); } continue; @@ -373,42 +345,7 @@ export class ChatTranscriptBuilder { #appendCustomMessage(message: Extract): void { if (!message.display) return; if (message.customType === "async-result") { - const details = ( - message as CustomMessage<{ - jobId?: string; - type?: "bash" | "task"; - label?: string; - durationMs?: number; - jobs?: Array<{ jobId?: string; type?: "bash" | "task"; label?: string; durationMs?: number }>; - }> - ).details; - const jobs = - details?.jobs && details.jobs.length > 0 - ? details.jobs - : [ - { - jobId: details?.jobId, - type: details?.type, - label: details?.label, - durationMs: details?.durationMs, - }, - ]; - const block = new TranscriptBlock(); - for (const job of jobs) { - const jobId = job.jobId ?? "unknown"; - const typeLabel = job.type ? `[${job.type}]` : "[job]"; - const duration = typeof job.durationMs === "number" ? formatDuration(job.durationMs) : undefined; - const line = [ - theme.fg("success", `${theme.status.done} Background job completed`), - theme.fg("dim", typeLabel), - theme.fg("accent", jobId), - duration ? theme.fg("dim", `(${duration})`) : undefined, - ] - .filter(Boolean) - .join(" "); - block.addChild(new Text(line, 1, 0)); - } - this.container.addChild(block); + this.container.addChild(buildAsyncResultBlock(message)); return; } if (message.customType === LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE) { @@ -433,28 +370,7 @@ export class ChatTranscriptBuilder { message.customType === "irc:autoreply" || message.customType === "irc:relay" ) { - const details = ( - message as CustomMessage<{ from?: string; to?: string; message?: string; body?: string; replyTo?: string }> - ).details; - const kind = - message.customType === "irc:incoming" - ? ("incoming" as const) - : message.customType === "irc:autoreply" - ? ("autoreply" as const) - : ("relay" as const); - const card = createIrcMessageCard( - { - kind, - from: details?.from, - to: details?.to, - body: kind === "incoming" ? details?.message : details?.body, - replyTo: details?.replyTo, - timestamp: message.timestamp, - }, - () => this.#expanded, - theme, - ); - this.container.addChild(card); + this.container.addChild(buildIrcMessageCard(message, () => this.#expanded)); return; } if (message.customType === "advisor") { diff --git a/packages/coding-agent/src/modes/components/custom-editor.ts b/packages/coding-agent/src/modes/components/custom-editor.ts index c917357d2..ee567a477 100644 --- a/packages/coding-agent/src/modes/components/custom-editor.ts +++ b/packages/coding-agent/src/modes/components/custom-editor.ts @@ -1,3 +1,4 @@ +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { addKeyAliases, canonicalKeyId, Editor, type KeyId, parseKey, parseKittySequence } from "@oh-my-pi/pi-tui"; import type { AppKeybinding } from "../../config/keybindings"; import { isSettingsInitialized, settings } from "../../config/settings"; @@ -163,6 +164,24 @@ export function extractBracketedImagePastePath(data: string): string | undefined export class CustomEditor extends Editor { imageLinks?: readonly (string | undefined)[]; + /** Draft images pasted into the composer, consumed on submit. Co-located with + * {@link imageLinks} so every piece of draft-image state lives on the editor. */ + pendingImages: ImageContent[] = []; + /** Per-image source links (file:// targets) parallel to {@link pendingImages}; + * `undefined` entries are images without a backing reference yet. */ + pendingImageLinks: (string | undefined)[] = []; + + /** Clear the composer draft: optionally commit `historyText` to history, then + * reset the editor text and all pending draft-image state. The shared tail of + * every "message submitted" path; pass no argument for a plain discard. */ + clearDraft(historyText?: string): void { + if (historyText !== undefined) this.addToHistory(historyText); + this.setText(""); + this.imageLinks = undefined; + this.pendingImages = []; + this.pendingImageLinks = []; + } + /** Treat image/paste markers as indivisible: a stray backspace deletes the whole token * instead of corrupting `[Paste #1, +30 lines]` into plain text. */ override atomicTokenPattern = PLACEHOLDER_REGEX; diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index bc8a339dd..f13186055 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -5,7 +5,7 @@ import { INTENT_FIELD } from "@oh-my-pi/pi-wire"; import { extractTextContent } from "../../commit/utils"; import { settings } from "../../config/settings"; import { getFileSnapshotStore } from "../../edit/file-snapshot-store"; -import { AssistantMessageComponent } from "../../modes/components/assistant-message"; +import type { AssistantMessageComponent } from "../../modes/components/assistant-message"; import { detectCacheInvalidation } from "../../modes/components/cache-invalidation-marker"; import { ReadToolGroupComponent, @@ -25,6 +25,7 @@ import type { ResolveToolDetails } from "../../tools/resolve"; import { vocalizer } from "../../tts/vocalizer"; import { canonicalizeMessage } from "../../utils/thinking-display"; import { interruptHint } from "../shared"; +import { createAssistantMessageComponent } from "../utils/interactive-context-helpers"; import { StreamingRevealController } from "./streaming-reveal"; import { ToolArgsRevealController } from "./tool-args-reveal"; @@ -332,14 +333,7 @@ export class EventController { this.ctx.ui.requestRender(); } else if (event.message.role === "assistant") { this.#lastVisibleBlockCount = 0; - this.ctx.streamingComponent = new AssistantMessageComponent( - undefined, - this.ctx.hideThinkingBlock, - () => this.ctx.ui.requestRender(), - this.ctx.viewSession.extensionRunner?.getAssistantThinkingRenderers(), - this.ctx.ui.imageBudget, - this.ctx.proseOnlyThinking, - ); + this.ctx.streamingComponent = createAssistantMessageComponent(this.ctx); this.ctx.streamingMessage = event.message; this.ctx.chatContainer.addChild(this.ctx.streamingComponent); this.#streamingReveal.begin(this.ctx.streamingComponent, this.ctx.streamingMessage); diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index 368974a2d..88c54933a 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -530,10 +530,7 @@ export class InputController { // model continues the prior intent rather than second-guessing the interrupt. if (text === "." || text === "c") { if (this.ctx.onInputCallback) { - this.ctx.editor.setText(""); - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; - this.ctx.editor.imageLinks = undefined; + this.ctx.editor.clearDraft(); this.ctx.onInputCallback({ text: manualContinuePrompt, cancelled: false, @@ -546,16 +543,14 @@ export class InputController { } const runner = this.ctx.session.extensionRunner; - let inputImages = this.ctx.pendingImages.length > 0 ? [...this.ctx.pendingImages] : undefined; - let inputImageLinks = this.ctx.pendingImageLinks.length > 0 ? [...this.ctx.pendingImageLinks] : undefined; + let inputImages = this.ctx.editor.pendingImages.length > 0 ? [...this.ctx.editor.pendingImages] : undefined; + let inputImageLinks = + this.ctx.editor.pendingImageLinks.length > 0 ? [...this.ctx.editor.pendingImageLinks] : undefined; if (runner?.hasHandlers("input")) { const result = await runner.emitInput(text, inputImages, "interactive"); if (result?.handled) { - this.ctx.editor.setText(""); - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; - this.ctx.editor.imageLinks = undefined; + this.ctx.editor.clearDraft(); return; } if (result?.text !== undefined) { @@ -603,12 +598,8 @@ export class InputController { this.ctx.showStatus("This collab link is read-only — prompting is disabled"); return; } - this.ctx.editor.addToHistory(text); - this.ctx.editor.setText(""); - this.ctx.editor.imageLinks = undefined; const images = inputImages && inputImages.length > 0 ? [...inputImages] : undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.clearDraft(text); // No local render: the prompt comes back from the host as a // collab-prompt event/entry and renders with the author badge. this.ctx.collabGuest.sendPrompt(text, images); @@ -680,8 +671,8 @@ export class InputController { this.ctx.editor.setText(""); this.ctx.editor.imageLinks = undefined; const images = inputImages && inputImages.length > 0 ? [...inputImages] : undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.pendingImages = []; + this.ctx.editor.pendingImageLinks = []; // Record the signature so the queued message's eventual delivery // (a user-role `message_start` event) leaves any draft the user has // typed since queuing intact. Same protection as #783, applied to @@ -744,8 +735,8 @@ export class InputController { // Include any pending images from clipboard paste this.ctx.editor.imageLinks = undefined; const images = inputImages && inputImages.length > 0 ? [...inputImages] : undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.pendingImages = []; + this.ctx.editor.pendingImageLinks = []; // Render user message immediately, then let session events catch up. // Tag the submission as "steer": this is a normal Enter the controller @@ -771,8 +762,8 @@ export class InputController { // semantics instead of throwing AgentBusyError. this.ctx.editor.imageLinks = undefined; const images = inputImages && inputImages.length > 0 ? [...inputImages] : undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.pendingImages = []; + this.ctx.editor.pendingImageLinks = []; try { await this.ctx.withLocalSubmission( text, @@ -787,9 +778,11 @@ export class InputController { // extension command). this.ctx.editor.setText(text); if (images && images.length > 0) { - this.ctx.pendingImages = [...images]; - this.ctx.pendingImageLinks = inputImageLinks ? [...inputImageLinks] : images.map(() => undefined); - this.ctx.editor.imageLinks = this.ctx.pendingImageLinks; + this.ctx.editor.pendingImages = [...images]; + this.ctx.editor.pendingImageLinks = inputImageLinks + ? [...inputImageLinks] + : images.map(() => undefined); + this.ctx.editor.imageLinks = this.ctx.editor.pendingImageLinks; } this.ctx.showError(error instanceof Error ? error.message : String(error)); } @@ -816,12 +809,8 @@ export class InputController { this.ctx.showStatus("Commands run in the main session — press ←← to return first"); return; // editor text not cleared: Editor does not auto-clear on submit } - const images = this.ctx.pendingImages.length > 0 ? [...this.ctx.pendingImages] : undefined; - this.ctx.editor.addToHistory(text); - this.ctx.editor.setText(""); - this.ctx.editor.imageLinks = undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + const images = this.ctx.editor.pendingImages.length > 0 ? [...this.ctx.editor.pendingImages] : undefined; + this.ctx.editor.clearDraft(text); try { // prompt() handles idle (new turn) and streaming (queues per streamingBehavior). await this.ctx.withLocalSubmission(text, () => target.prompt(text, { streamingBehavior, images }), { @@ -989,10 +978,7 @@ export class InputController { } const didRetry = await this.ctx.viewSession.retry(); if (didRetry) { - this.ctx.editor.setText(""); - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; - this.ctx.editor.imageLinks = undefined; + this.ctx.editor.clearDraft(); } else { this.ctx.showStatus("Nothing to retry"); } @@ -1016,7 +1002,7 @@ export class InputController { // the queued entry is later re-parsed into a skill invocation is a // separate concern owned by the compaction-resume path. if (this.ctx.session.isCompacting) { - const images = this.ctx.pendingImages.length > 0 ? [...this.ctx.pendingImages] : undefined; + const images = this.ctx.editor.pendingImages.length > 0 ? [...this.ctx.editor.pendingImages] : undefined; this.ctx.queueCompactionMessage(text, "followUp", images); return; } @@ -1040,14 +1026,10 @@ export class InputController { // Forward any pending clipboard-pasted images alongside the queued text; // otherwise the follow-up would drop the image (mirrors the Enter/steer path). - const images = this.ctx.pendingImages.length > 0 ? [...this.ctx.pendingImages] : undefined; + const images = this.ctx.editor.pendingImages.length > 0 ? [...this.ctx.editor.pendingImages] : undefined; if (this.ctx.session.isStreaming) { - this.ctx.editor.addToHistory(text); - this.ctx.editor.setText(""); - this.ctx.editor.imageLinks = undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.clearDraft(text); await this.ctx.withLocalSubmission( text, () => this.ctx.session.prompt(text, { streamingBehavior: "followUp", images }), @@ -1059,11 +1041,7 @@ export class InputController { } // Not streaming — just submit normally - this.ctx.editor.addToHistory(text); - this.ctx.editor.setText(""); - this.ctx.editor.imageLinks = undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.clearDraft(text); await this.ctx.withLocalSubmission(text, () => this.ctx.session.prompt(text, { images }), { imageCount: images?.length ?? 0, }); @@ -1108,7 +1086,7 @@ export class InputController { let queuedText: string; if (queuedImages.length > 0) { const parts: string[] = []; - let imageOffset = this.ctx.pendingImages.length; + let imageOffset = this.ctx.editor.pendingImages.length; for (const entry of allQueued) { parts.push(shiftImageMarkers(entry.text, imageOffset)); if (entry.images && entry.images.length > 0) imageOffset += entry.images.length; @@ -1124,9 +1102,9 @@ export class InputController { // re-materialized lazily; the restored text already carries the // renumbered `[Image #N, WxH]` markers). if (queuedImages.length > 0) { - this.ctx.pendingImages.push(...queuedImages); - this.ctx.pendingImageLinks.push(...queuedImages.map(() => undefined)); - this.ctx.editor.imageLinks = this.ctx.pendingImageLinks; + this.ctx.editor.pendingImages.push(...queuedImages); + this.ctx.editor.pendingImageLinks.push(...queuedImages.map(() => undefined)); + this.ctx.editor.imageLinks = this.ctx.editor.pendingImageLinks; } this.ctx.updatePendingMessagesDisplay(); if (options?.abort) { @@ -1148,14 +1126,14 @@ export class InputController { this.ctx.sessionManager.putBlob.bind(this.ctx.sessionManager), ) )?.[0]; - this.ctx.pendingImages.push({ + this.ctx.editor.pendingImages.push({ type: "image", data: imageData.data, mimeType: imageData.mimeType, }); - this.ctx.pendingImageLinks.push(imageLink); - this.ctx.editor.imageLinks = this.ctx.pendingImageLinks; - const imageNum = this.ctx.pendingImages.length; + this.ctx.editor.pendingImageLinks.push(imageLink); + this.ctx.editor.imageLinks = this.ctx.editor.pendingImageLinks; + const imageNum = this.ctx.editor.pendingImages.length; const dims = await this.#imageDimensions(imageData); const label = dims ? `[Image #${imageNum}, ${dims.width}x${dims.height}]` : `[Image #${imageNum}]`; this.ctx.editor.insertText(`${label} `); diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index bc0e11957..453305298 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -408,8 +408,6 @@ export class InteractiveMode implements InteractiveModeContext { todoPhases: TodoPhase[] = []; hideThinkingBlock = false; proseOnlyThinking = true; - pendingImages: ImageContent[] = []; - pendingImageLinks: (string | undefined)[] = []; compactionQueuedMessages: CompactionQueuedMessage[] = []; pendingTools = new Map(); pendingBashComponents: BashExecutionComponent[] = []; @@ -1094,7 +1092,7 @@ export class InteractiveMode implements InteractiveModeContext { if (this.#goalSuppressNextContinuation) return; if (this.#pendingSubmittedInput) return; if (this.editor.getText().trim().length > 0) return; - if ((this.pendingImages?.length ?? 0) > 0) return; + if ((this.editor.pendingImages?.length ?? 0) > 0) return; const state = this.session.getGoalModeState(); if (!state?.enabled || state.goal.status !== "active") return; const prompt = this.session.goalRuntime.buildContinuationPrompt(); @@ -1113,7 +1111,7 @@ export class InteractiveMode implements InteractiveModeContext { if (this.#isAutoSubmitBlocked()) return; if (this.#pendingSubmittedInput) return; if (this.editor.getText().trim().length > 0) return; - if ((this.pendingImages?.length ?? 0) > 0) return; + if ((this.editor.pendingImages?.length ?? 0) > 0) return; const latestState = this.session.getGoalModeState(); if (!latestState?.enabled || latestState.goal.status !== "active") return; this.#goalContinuationTurnInFlight = true; @@ -1338,9 +1336,9 @@ export class InteractiveMode implements InteractiveModeContext { this.#stopLoadingAnimation(true); } if (!submission.customType) { - this.pendingImages = submission.images ? [...submission.images] : []; - this.pendingImageLinks = submission.imageLinks ? [...submission.imageLinks] : []; - this.editor.imageLinks = this.pendingImageLinks; + this.editor.pendingImages = submission.images ? [...submission.images] : []; + this.editor.pendingImageLinks = submission.imageLinks ? [...submission.imageLinks] : []; + this.editor.imageLinks = this.editor.pendingImageLinks; this.rebuildChatFromMessages(); this.editor.setText(submission.text); } diff --git a/packages/coding-agent/src/modes/types.ts b/packages/coding-agent/src/modes/types.ts index 01fd9571e..731b64a84 100644 --- a/packages/coding-agent/src/modes/types.ts +++ b/packages/coding-agent/src/modes/types.ts @@ -160,8 +160,6 @@ export interface InteractiveModeContext { planModePlanFilePath?: string; hideThinkingBlock: boolean; proseOnlyThinking: boolean; - pendingImages: ImageContent[]; - pendingImageLinks: (string | undefined)[]; compactionQueuedMessages: CompactionQueuedMessage[]; pendingTools: Map; pendingBashComponents: BashExecutionComponent[]; diff --git a/packages/coding-agent/src/modes/utils/interactive-context-helpers.ts b/packages/coding-agent/src/modes/utils/interactive-context-helpers.ts new file mode 100644 index 000000000..80a6d9e18 --- /dev/null +++ b/packages/coding-agent/src/modes/utils/interactive-context-helpers.ts @@ -0,0 +1,27 @@ +/** + * Small helpers over {@link InteractiveModeContext} shared between + * {@link UiHelpers} and the input/event controllers, so the live chat surfaces + * construct components and reset editor state identically. + */ +import type { AssistantMessage } from "@oh-my-pi/pi-ai"; +import { AssistantMessageComponent } from "../components/assistant-message"; +import type { InteractiveModeContext } from "../types"; + +/** + * Construct an {@link AssistantMessageComponent} wired to the live context's + * thinking/image settings. `message` is omitted for the streaming placeholder + * component and supplied when rendering a persisted turn. + */ +export function createAssistantMessageComponent( + ctx: InteractiveModeContext, + message?: AssistantMessage, +): AssistantMessageComponent { + return new AssistantMessageComponent( + message, + ctx.hideThinkingBlock, + () => ctx.ui.requestRender(), + ctx.viewSession.extensionRunner?.getAssistantThinkingRenderers(), + ctx.ui.imageBudget, + ctx.proseOnlyThinking, + ); +} diff --git a/packages/coding-agent/src/modes/utils/transcript-render-helpers.ts b/packages/coding-agent/src/modes/utils/transcript-render-helpers.ts new file mode 100644 index 000000000..b313b86e8 --- /dev/null +++ b/packages/coding-agent/src/modes/utils/transcript-render-helpers.ts @@ -0,0 +1,157 @@ +/** + * Render helpers shared between the live transcript ({@link UiHelpers}) and the + * file/remote-backed {@link ChatTranscriptBuilder}. Both surfaces build the same + * transcript rows from persisted message entries; holding the row construction + * here keeps the two byte-for-byte identical. + */ +import type { AgentMessage } from "@oh-my-pi/pi-agent-core"; +import { type Component, Text } from "@oh-my-pi/pi-tui"; +import { formatBytes, formatDuration } from "@oh-my-pi/pi-utils"; +import { type CustomMessage, type FileMentionMessage, isSilentAbort, resolveAbortLabel } from "../../session/messages"; +import { createIrcMessageCard } from "../../tools/irc"; +import { canonicalizeMessage } from "../../utils/thinking-display"; +import { TranscriptBlock } from "../components/transcript-container"; +import { theme } from "../theme/theme"; + +type CustomOrHookMessage = Extract; +type AssistantAgentMessage = Extract; + +/** + * Render an `async-result` custom message (a completed background bash/task job, + * or a batch of them) as a transcript block of one "Background job completed" + * row per job. + */ +export function buildAsyncResultBlock(message: CustomOrHookMessage): TranscriptBlock { + const details = ( + message as CustomMessage<{ + jobId?: string; + type?: "bash" | "task"; + label?: string; + durationMs?: number; + jobs?: Array<{ jobId?: string; type?: "bash" | "task"; label?: string; durationMs?: number }>; + }> + ).details; + const jobs = + details?.jobs && details.jobs.length > 0 + ? details.jobs + : [ + { + jobId: details?.jobId, + type: details?.type, + label: details?.label, + durationMs: details?.durationMs, + }, + ]; + const block = new TranscriptBlock(); + for (const job of jobs) { + const jobId = job.jobId ?? "unknown"; + const typeLabel = job.type ? `[${job.type}]` : "[job]"; + const duration = typeof job.durationMs === "number" ? formatDuration(job.durationMs) : undefined; + const line = [ + theme.fg("success", `${theme.status.done} Background job completed`), + theme.fg("dim", typeLabel), + theme.fg("accent", jobId), + duration ? theme.fg("dim", `(${duration})`) : undefined, + ] + .filter(Boolean) + .join(" "); + block.addChild(new Text(line, 1, 0)); + } + return block; +} + +/** + * Render a live IRC traffic custom message (`irc:incoming` / `irc:autoreply` / + * `irc:relay`) as a transcript card. `getExpanded` supplies the live + * expanded-state getter for the cached card. + */ +export function buildIrcMessageCard(message: CustomOrHookMessage, getExpanded: () => boolean): Component { + const details = ( + message as CustomMessage<{ from?: string; to?: string; message?: string; body?: string; replyTo?: string }> + ).details; + const kind = + message.customType === "irc:incoming" + ? ("incoming" as const) + : message.customType === "irc:autoreply" + ? ("autoreply" as const) + : ("relay" as const); + return createIrcMessageCard( + { + kind, + from: details?.from, + to: details?.to, + body: kind === "incoming" ? details?.message : details?.body, + replyTo: details?.replyTo, + timestamp: message.timestamp, + }, + getExpanded, + theme, + ); +} + +/** + * Render a `fileMention` message's files as a transcript block of "Read " + * rows. `indent` sets the left pad: the live chat renders within an outer gutter + * (0), the transcript viewer renders body rows without one so rows own their pad + * (1). + */ +export function buildFileMentionBlock(files: FileMentionMessage["files"], indent: number): TranscriptBlock { + const block = new TranscriptBlock(); + for (const file of files) { + let suffix: string; + if (file.skippedReason === "tooLarge") { + const size = typeof file.byteSize === "number" ? formatBytes(file.byteSize) : "unknown size"; + suffix = `(skipped: ${size})`; + } else { + suffix = file.image + ? "(image)" + : file.lineCount === undefined + ? "(unknown lines)" + : `(${file.lineCount} lines)`; + } + const text = `${theme.fg("dim", `${theme.tree.last} `)}${theme.fg("muted", "Read")} ${theme.fg( + "accent", + file.path, + )} ${theme.fg("dim", suffix)}`; + block.addChild(new Text(text, indent, 0)); + } + return block; +} + +/** + * Whether an assistant turn has visible text or thinking content (after + * canonicalization) — i.e. content that closes the current read-tool run. + */ +export function assistantHasVisibleContent(message: AssistantAgentMessage): boolean { + return message.content.some( + content => + (content.type === "text" && canonicalizeMessage(content.text)) || + (content.type === "thinking" && canonicalizeMessage(content.thinking)), + ); +} + +/** + * Normalize raw tool-call arguments to a plain record, collapsing non-object or + * array values to an empty object. + */ +export function normalizeToolArgs(args: unknown): Record { + return args && typeof args === "object" && !Array.isArray(args) ? (args as Record) : {}; +} + +/** + * Resolve the inline error label, if any, for a turn-ending assistant message. + * Silent aborts yield no label. `retryAttempt` tunes the abort label wording. + */ +export function resolveAssistantErrorMessage( + message: AssistantAgentMessage, + retryAttempt = 0, +): { hasErrorStop: boolean; errorMessage: string | null } { + const isAbortedSilently = message.stopReason === "aborted" && isSilentAbort(message.errorMessage); + const hasErrorStop = !isAbortedSilently && (message.stopReason === "aborted" || message.stopReason === "error"); + const errorMessage = hasErrorStop + ? message.stopReason === "aborted" + ? resolveAbortLabel(message.errorMessage, retryAttempt) + : message.errorMessage || "Error" + : null; + return { hasErrorStop, errorMessage }; +} diff --git a/packages/coding-agent/src/modes/utils/ui-helpers.ts b/packages/coding-agent/src/modes/utils/ui-helpers.ts index b0ca4ac90..2be0c90f4 100644 --- a/packages/coding-agent/src/modes/utils/ui-helpers.ts +++ b/packages/coding-agent/src/modes/utils/ui-helpers.ts @@ -39,16 +39,20 @@ import type { CompactionQueuedMessage, InteractiveModeContext } from "../../mode import { BACKGROUND_TAN_DISPATCH_MESSAGE_TYPE, type CustomMessage, - isSilentAbort, LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE, - resolveAbortLabel, SKILL_PROMPT_MESSAGE_TYPE, type SkillPromptDetails, } from "../../session/messages"; import type { SessionContext } from "../../session/session-context"; -import { createIrcMessageCard } from "../../tools/irc"; -import { formatBytes, formatDuration } from "../../tools/render-utils"; -import { canonicalizeMessage } from "../../utils/thinking-display"; +import { createAssistantMessageComponent } from "./interactive-context-helpers"; +import { + assistantHasVisibleContent, + buildAsyncResultBlock, + buildFileMentionBlock, + buildIrcMessageCard, + normalizeToolArgs, + resolveAssistantErrorMessage, +} from "./transcript-render-helpers"; type TextBlock = { type: "text"; text: string }; interface RenderInitialMessagesOptions { @@ -143,47 +147,7 @@ export class UiHelpers { case "custom": { if (message.display) { if (message.customType === "async-result") { - const details = ( - message as CustomMessage<{ - jobId?: string; - type?: "bash" | "task"; - label?: string; - durationMs?: number; - jobs?: Array<{ - jobId?: string; - type?: "bash" | "task"; - label?: string; - durationMs?: number; - }>; - }> - ).details; - const jobs = - details?.jobs && details.jobs.length > 0 - ? details.jobs - : [ - { - jobId: details?.jobId, - type: details?.type, - label: details?.label, - durationMs: details?.durationMs, - }, - ]; - const block = new TranscriptBlock(); - for (const job of jobs) { - const jobId = job.jobId ?? "unknown"; - const typeLabel = job.type ? `[${job.type}]` : "[job]"; - const duration = typeof job.durationMs === "number" ? formatDuration(job.durationMs) : undefined; - const line = [ - theme.fg("success", `${theme.status.done} Background job completed`), - theme.fg("dim", typeLabel), - theme.fg("accent", jobId), - duration ? theme.fg("dim", `(${duration})`) : undefined, - ] - .filter(Boolean) - .join(" "); - block.addChild(new Text(line, 1, 0)); - } - this.ctx.chatContainer.addChild(block); + this.ctx.chatContainer.addChild(buildAsyncResultBlock(message)); break; } if (message.customType === LSP_LATE_DIAGNOSTIC_MESSAGE_TYPE) { @@ -213,33 +177,7 @@ export class UiHelpers { message.customType === "irc:autoreply" || message.customType === "irc:relay" ) { - const details = ( - message as CustomMessage<{ - from?: string; - to?: string; - message?: string; - body?: string; - replyTo?: string; - }> - ).details; - const kind = - message.customType === "irc:incoming" - ? ("incoming" as const) - : message.customType === "irc:autoreply" - ? ("autoreply" as const) - : ("relay" as const); - const card = createIrcMessageCard( - { - kind, - from: details?.from, - to: details?.to, - body: kind === "incoming" ? details?.message : details?.body, - replyTo: details?.replyTo, - timestamp: message.timestamp, - }, - () => this.ctx.toolOutputExpanded, - theme, - ); + const card = buildIrcMessageCard(message, () => this.ctx.toolOutputExpanded); this.ctx.chatContainer.addChild(card); return [card]; } @@ -284,25 +222,7 @@ export class UiHelpers { } case "fileMention": { // Render compact file mention display - const block = new TranscriptBlock(); - for (const file of message.files) { - let suffix: string; - if (file.skippedReason === "tooLarge") { - const size = typeof file.byteSize === "number" ? formatBytes(file.byteSize) : "unknown size"; - suffix = `(skipped: ${size})`; - } else { - suffix = file.image - ? "(image)" - : file.lineCount === undefined - ? "(unknown lines)" - : `(${file.lineCount} lines)`; - } - const text = `${theme.fg("dim", `${theme.tree.last} `)}${theme.fg("muted", "Read")} ${theme.fg( - "accent", - file.path, - )} ${theme.fg("dim", suffix)}`; - block.addChild(new Text(text, 0, 0)); - } + const block = buildFileMentionBlock(message.files, 0); if (block.children.length > 0) this.ctx.chatContainer.addChild(block); break; } @@ -326,14 +246,7 @@ export class UiHelpers { break; } case "assistant": { - const assistantComponent = new AssistantMessageComponent( - message, - this.ctx.hideThinkingBlock, - () => this.ctx.ui.requestRender(), - this.ctx.viewSession.extensionRunner?.getAssistantThinkingRenderers(), - this.ctx.ui.imageBudget, - this.ctx.proseOnlyThinking, - ); + const assistantComponent = createAssistantMessageComponent(this.ctx, message); this.ctx.chatContainer.addChild(assistantComponent); break; } @@ -425,11 +338,7 @@ export class UiHelpers { this.ctx.lastAssistantUsage = usage; } } - const hasVisibleAssistantContent = message.content.some( - content => - (content.type === "text" && canonicalizeMessage(content.text)) || - (content.type === "thinking" && canonicalizeMessage(content.thinking)), - ); + const hasVisibleAssistantContent = assistantHasVisibleContent(message); if (hasVisibleAssistantContent) { // Rebuild reconstructs immutable history; seal (not finalize) so the // group freezes even if a read's result was never persisted — @@ -438,14 +347,10 @@ export class UiHelpers { readGroup?.seal(); readGroup = null; } - const isAbortedSilently = message.stopReason === "aborted" && isSilentAbort(message.errorMessage); - const hasErrorStop = - !isAbortedSilently && (message.stopReason === "aborted" || message.stopReason === "error"); - const errorMessage = hasErrorStop - ? message.stopReason === "aborted" - ? resolveAbortLabel(message.errorMessage, this.ctx.viewSession.retryAttempt) - : message.errorMessage || "Error" - : null; + const { hasErrorStop, errorMessage } = resolveAssistantErrorMessage( + message, + this.ctx.viewSession.retryAttempt, + ); // Render tool call components for (const content of message.content) { @@ -474,10 +379,7 @@ export class UiHelpers { content.id, ); } else { - const normalizedArgs = - content.arguments && typeof content.arguments === "object" && !Array.isArray(content.arguments) - ? (content.arguments as Record) - : {}; + const normalizedArgs = normalizeToolArgs(content.arguments); readToolCallArgs.set(content.id, normalizedArgs); if (assistantComponent) { readToolCallAssistantComponents.set(content.id, assistantComponent); @@ -647,10 +549,7 @@ export class UiHelpers { } clearEditor(): void { - this.ctx.editor.setText(""); - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; - this.ctx.editor.imageLinks = undefined; + this.ctx.editor.clearDraft(); this.ctx.ui.requestRender(); } @@ -719,11 +618,7 @@ export class UiHelpers { queueCompactionMessage(text: string, mode: "steer" | "followUp", images?: ImageContent[]): void { const queuedImages = images && images.length > 0 ? images : undefined; this.ctx.compactionQueuedMessages.push({ text, mode, images: queuedImages } as CompactionQueuedMessage); - this.ctx.editor.addToHistory(text); - this.ctx.editor.setText(""); - this.ctx.editor.imageLinks = undefined; - this.ctx.pendingImages = []; - this.ctx.pendingImageLinks = []; + this.ctx.editor.clearDraft(text); this.ctx.updatePendingMessagesDisplay(); this.ctx.showStatus( queuedImages ? "Queued message with image for after compaction" : "Queued message for after compaction", diff --git a/packages/coding-agent/src/prompts/tools/find.md b/packages/coding-agent/src/prompts/tools/find.md index 5436dca0c..07d0c8b76 100644 --- a/packages/coding-agent/src/prompts/tools/find.md +++ b/packages/coding-agent/src/prompts/tools/find.md @@ -13,4 +13,3 @@ Matching paths sorted by mtime (newest first), grouped under `# /` headers Open-ended searches needing multiple rounds of globbing/searching: you MUST use the Task tool instead. - diff --git a/packages/coding-agent/test/input-controller-compaction-image.test.ts b/packages/coding-agent/test/input-controller-compaction-image.test.ts index b117292f8..a8618b721 100644 --- a/packages/coding-agent/test/input-controller-compaction-image.test.ts +++ b/packages/coding-agent/test/input-controller-compaction-image.test.ts @@ -62,16 +62,22 @@ function makeCtx(initialQueue: CompactionQueuedMessage[] = []) { const ctx = { session, compactionQueuedMessages: [...initialQueue], - pendingImages: [] as ImageContent[], - pendingImageLinks: [] as (string | undefined)[], pendingMessagesContainer: { clear: () => {}, addChild: () => {}, removeChild: () => {} }, editor: { addToHistory: () => {}, + clearDraft: (_historyText?: string) => { + editorText = ""; + ctx.editor.pendingImages = []; + ctx.editor.pendingImageLinks = []; + ctx.editor.imageLinks = undefined; + }, setText: (text: string) => { editorText = text; }, getText: () => editorText, imageLinks: undefined as (string | undefined)[] | undefined, + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], }, keybindings: { getDisplayString: () => "Alt+Up" }, fileSlashCommands: new Set(), @@ -95,8 +101,8 @@ describe("compaction queue image forwarding", () => { test("queueCompactionMessage stores images and consumes pending-image state", () => { const image = img("aGVsbG8="); const { ctx } = makeCtx(); - ctx.pendingImages = [image]; - ctx.pendingImageLinks = ["clipboard"]; + ctx.editor.pendingImages = [image]; + ctx.editor.pendingImageLinks = ["clipboard"]; ctx.editor.imageLinks = ["clipboard"]; new UiHelpers(ctx).queueCompactionMessage("look at this screenshot", "steer", [image]); @@ -105,8 +111,8 @@ describe("compaction queue image forwarding", () => { { text: "look at this screenshot", mode: "steer", images: [image] }, ]); // Pending state is consumed so the next message does not resend the image. - expect(ctx.pendingImages).toEqual([]); - expect(ctx.pendingImageLinks).toEqual([]); + expect(ctx.editor.pendingImages).toEqual([]); + expect(ctx.editor.pendingImageLinks).toEqual([]); expect(ctx.editor.imageLinks).toBeUndefined(); }); @@ -153,7 +159,7 @@ describe("compaction queue Alt+Up restore", () => { const { ctx } = makeCtx([{ text: "look", mode: "steer", images: [image] }]); const restored = new InputController(ctx).restoreQueuedMessagesToEditor(); expect(restored).toBe(1); - expect(ctx.pendingImages).toEqual([image]); + expect(ctx.editor.pendingImages).toEqual([image]); }); test("session and compaction queues restore in pending-bar order", () => { @@ -192,7 +198,15 @@ describe("restoreQueuedMessagesToEditor image marker alignment", () => { }, getText: () => editorText, addToHistory: () => {}, + clearDraft: (_historyText?: string) => { + editorText = ""; + editor.pendingImages = []; + editor.pendingImageLinks = []; + editor.imageLinks = undefined; + }, imageLinks: undefined as (string | undefined)[] | undefined, + pendingImages: opts.draftImages ? [...opts.draftImages] : ([] as ImageContent[]), + pendingImageLinks: opts.draftImages ? opts.draftImages.map(() => undefined) : ([] as (string | undefined)[]), }; const session = { clearQueue: mock(() => ({ steering: opts.queued ?? [], followUp: [] })), @@ -201,8 +215,6 @@ describe("restoreQueuedMessagesToEditor image marker alignment", () => { const ctx = { session, editor, - pendingImages: opts.draftImages ? [...opts.draftImages] : ([] as ImageContent[]), - pendingImageLinks: opts.draftImages ? opts.draftImages.map(() => undefined) : ([] as (string | undefined)[]), compactionQueuedMessages: [], locallySubmittedUserSignatures: new Set(), updatePendingMessagesDisplay: () => {}, @@ -225,10 +237,10 @@ describe("restoreQueuedMessagesToEditor image marker alignment", () => { // marker is bumped to #2 because the queued image is appended at slot 1. expect(restored).toBe(1); expect(editor.getText()).toBe("[Image #2] queued text\n\n[Image #1] draft text"); - expect(ctx.pendingImages).toEqual([draftImg, queuedImg]); + expect(ctx.editor.pendingImages).toEqual([draftImg, queuedImg]); // Marker → image positional mapping after restore. - expect(ctx.pendingImages[0]).toBe(draftImg); // matches [Image #1] - expect(ctx.pendingImages[1]).toBe(queuedImg); // matches [Image #2] + expect(ctx.editor.pendingImages[0]).toBe(draftImg); // matches [Image #1] + expect(ctx.editor.pendingImages[1]).toBe(queuedImg); // matches [Image #2] }); test("preserves the WxH metadata tail when renumbering", () => { @@ -263,7 +275,7 @@ describe("restoreQueuedMessagesToEditor image marker alignment", () => { // msg1 markers shift by 1 (draft images), msg2 markers shift by 1+1=2. expect(editor.getText()).toBe("first [Image #2]\n\nsecond [Image #3] and [Image #4]\n\nsee [Image #1]"); - expect(ctx.pendingImages).toEqual([draftImg, queued1, queued2a, queued2b]); + expect(ctx.editor.pendingImages).toEqual([draftImg, queued1, queued2a, queued2b]); }); test("leaves the queued text untouched when the draft has no pending images", () => { @@ -275,6 +287,6 @@ describe("restoreQueuedMessagesToEditor image marker alignment", () => { new InputController(ctx).restoreQueuedMessagesToEditor(); expect(editor.getText()).toBe("[Image #1] queued"); - expect(ctx.pendingImages).toEqual([queuedImg]); + expect(ctx.editor.pendingImages).toEqual([queuedImg]); }); }); diff --git a/packages/coding-agent/test/input-controller-escape.test.ts b/packages/coding-agent/test/input-controller-escape.test.ts index 3e7befad0..ac29fdb2b 100644 --- a/packages/coding-agent/test/input-controller-escape.test.ts +++ b/packages/coding-agent/test/input-controller-escape.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "bun:test"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller"; import type { InteractiveModeContext, SubmittedUserInput } from "@oh-my-pi/pi-coding-agent/modes/types"; @@ -32,12 +33,14 @@ type FakeEditor = { setActionKeys(action: string, keys: string[]): void; setCustomKeyHandler(key: string, handler: () => void): void; clearCustomKeyHandlers(): void; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; }; function createSubmission(input: { text: string; - images?: InteractiveModeContext["pendingImages"]; - imageLinks?: InteractiveModeContext["pendingImageLinks"]; + images?: ImageContent[]; + imageLinks?: (string | undefined)[]; }): SubmittedUserInput { return { text: input.text, @@ -99,11 +102,7 @@ function createContext(): { const updatePendingMessagesDisplay = vi.fn(); const prompt = vi.fn(); const startPendingSubmission = vi.fn( - (input: { - text: string; - images?: InteractiveModeContext["pendingImages"]; - imageLinks?: InteractiveModeContext["pendingImageLinks"]; - }) => { + (input: { text: string; images?: ImageContent[]; imageLinks?: (string | undefined)[] }) => { ensureLoadingAnimation(); return createSubmission(input); }, @@ -119,6 +118,8 @@ function createContext(): { setActionKeys: vi.fn(), setCustomKeyHandler: vi.fn(), clearCustomKeyHandlers: vi.fn(), + pendingImages: [], + pendingImageLinks: [], }; let ctx!: InteractiveModeContext; @@ -173,8 +174,6 @@ function createContext(): { keybindings: { getKeys: () => [], } as unknown as InteractiveModeContext["keybindings"], - pendingImages: [], - pendingImageLinks: [], compactionQueuedMessages: [], isBashMode: false, isPythonMode: false, diff --git a/packages/coding-agent/test/input-controller-followup-image.test.ts b/packages/coding-agent/test/input-controller-followup-image.test.ts index 5410fd763..7974178fe 100644 --- a/packages/coding-agent/test/input-controller-followup-image.test.ts +++ b/packages/coding-agent/test/input-controller-followup-image.test.ts @@ -15,6 +15,9 @@ interface StubEditor { getText: () => string; addToHistory: (text: string) => void; imageLinks?: unknown; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; + clearDraft: (text?: string) => void; } interface PromptOptionsLike { streamingBehavior?: "steer" | "followUp"; @@ -31,6 +34,15 @@ function createContext(opts: { isStreaming: boolean; pendingImages: ImageContent return editorText; }, addToHistory: vi.fn(), + pendingImages: opts.pendingImages, + pendingImageLinks: opts.pendingImages.map(() => undefined), + clearDraft(text?: string) { + if (text !== undefined) this.addToHistory(text); + this.setText(""); + this.imageLinks = undefined; + this.pendingImages = []; + this.pendingImageLinks = []; + }, }; const prompt = vi.fn(async (_text: string, _options?: PromptOptionsLike) => {}); const updatePendingMessagesDisplay = vi.fn(); @@ -48,8 +60,6 @@ function createContext(opts: { isStreaming: boolean; pendingImages: ImageContent extensionRunner: undefined, prompt, }, - pendingImages: opts.pendingImages, - pendingImageLinks: opts.pendingImages.map(() => undefined), loopModeEnabled: false, compactionQueuedMessages: [], locallySubmittedUserSignatures: new Set(), @@ -81,8 +91,8 @@ describe("InputController.handleFollowUp image forwarding", () => { expect(call[1]?.images).toEqual([image]); // Pending image state is consumed so the next message does not resend it. - expect(ctx.pendingImages).toEqual([]); - expect(ctx.pendingImageLinks).toEqual([]); + expect(ctx.editor.pendingImages).toEqual([]); + expect(ctx.editor.pendingImageLinks).toEqual([]); }); it("forwards pending images when not streaming", async () => { @@ -98,7 +108,7 @@ describe("InputController.handleFollowUp image forwarding", () => { if (!call) throw new Error("expected session.prompt to be called"); expect(call[1]?.images).toEqual([image]); expect(call[1]?.streamingBehavior).toBeUndefined(); - expect(ctx.pendingImages).toEqual([]); + expect(ctx.editor.pendingImages).toEqual([]); }); it("omits images when none are pending", async () => { diff --git a/packages/coding-agent/test/input-controller-keybindings.test.ts b/packages/coding-agent/test/input-controller-keybindings.test.ts index 70165376b..c212b5f99 100644 --- a/packages/coding-agent/test/input-controller-keybindings.test.ts +++ b/packages/coding-agent/test/input-controller-keybindings.test.ts @@ -32,6 +32,9 @@ type FakeEditor = { clearCustomKeyHandlers(): void; pasteText(text: string): void; imageLinks?: (string | undefined)[]; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; + clearDraft(historyText?: string): void; }; type InputListenerResult = { consume: boolean } | undefined; @@ -108,6 +111,15 @@ async function createContext() { setActionKeys, setCustomKeyHandler, clearCustomKeyHandlers, + pendingImages: [], + pendingImageLinks: [], + clearDraft(historyText?: string) { + if (historyText !== undefined) this.addToHistory(historyText); + this.setText(""); + this.imageLinks = undefined; + this.pendingImages = []; + this.pendingImageLinks = []; + }, }; focused = editor; const ctx = { @@ -132,7 +144,6 @@ async function createContext() { return keyMap[action] ? [...keyMap[action]] : []; }, } as InteractiveModeContext["keybindings"], - pendingImages: [], locallySubmittedUserSignatures: new Set(), isKnownSlashCommand: () => false, recordLocalSubmission(this: InteractiveModeContext, text: string, imageCount = 0) { @@ -304,15 +315,15 @@ describe("InputController keybinding setup", () => { const controller = new InputController(ctx); controller.setupKeyHandlers(); - ctx.pendingImages = [image]; - ctx.pendingImageLinks = ["local://draft.png"]; - editor.imageLinks = ctx.pendingImageLinks; + ctx.editor.pendingImages = [image]; + ctx.editor.pendingImageLinks = ["local://draft.png"]; + editor.imageLinks = ctx.editor.pendingImageLinks; editor.setText("draft with image"); editor.onRetry?.(); await Promise.resolve(); - expect(ctx.pendingImages).toEqual([]); - expect(ctx.pendingImageLinks).toEqual([]); + expect(ctx.editor.pendingImages).toEqual([]); + expect(ctx.editor.pendingImageLinks).toEqual([]); expect(editor.imageLinks).toBeUndefined(); expect(editor.getText()).toBe(""); }); diff --git a/packages/coding-agent/test/input-controller-orphan-submit.test.ts b/packages/coding-agent/test/input-controller-orphan-submit.test.ts index 02ce3e5c3..c71c52fe8 100644 --- a/packages/coding-agent/test/input-controller-orphan-submit.test.ts +++ b/packages/coding-agent/test/input-controller-orphan-submit.test.ts @@ -1,6 +1,7 @@ import { describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; @@ -27,6 +28,8 @@ import { TempDir } from "@oh-my-pi/pi-utils"; type FakeEditor = { onSubmit?: (text: string) => Promise; imageLinks?: readonly (string | undefined)[]; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; setText(text: string): void; getText(): string; addToHistory(text: string): void; @@ -46,6 +49,8 @@ function createContext(sessionOverride?: InteractiveModeContext["session"]) { const flushPendingBashComponents = vi.fn(); const editor: FakeEditor = { + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], setText(text: string) { editorText = text; }, @@ -77,8 +82,6 @@ function createContext(sessionOverride?: InteractiveModeContext["session"]) { ui: { requestRender } as unknown as InteractiveModeContext["ui"], session, sessionManager: { getSessionName: () => "named-session" } as InteractiveModeContext["sessionManager"], - pendingImages: [] as InteractiveModeContext["pendingImages"], - pendingImageLinks: [] as InteractiveModeContext["pendingImageLinks"], compactionQueuedMessages: [] as InteractiveModeContext["compactionQueuedMessages"], fileSlashCommands: new Set(), locallySubmittedUserSignatures: new Set(), @@ -184,7 +187,7 @@ describe("InputController orphaned submit", () => { it("forwards pending images and counts them in the local-submission signature", async () => { const { ctx, editor, spies } = createContext(); const image = { type: "image", data: "abc", mimeType: "image/png" }; - (ctx.pendingImages as unknown[]).push(image); + (ctx.editor.pendingImages as unknown[]).push(image); const controller = new InputController(ctx); controller.setupEditorSubmitHandler(); @@ -192,13 +195,13 @@ describe("InputController orphaned submit", () => { expect(spies.prompt).toHaveBeenCalledWith("look at this", { streamingBehavior: "steer", images: [image] }); expect(ctx.locallySubmittedUserSignatures.has("look at this\u00001")).toBe(true); - expect(ctx.pendingImages.length).toBe(0); + expect(ctx.editor.pendingImages.length).toBe(0); }); it("restores text and images to the editor when prompt dispatch rejects", async () => { const { ctx, editor, spies } = createContext(); const image = { type: "image" as const, data: "abc", mimeType: "image/png" }; - (ctx.pendingImages as unknown[]).push(image); + (ctx.editor.pendingImages as unknown[]).push(image); spies.prompt.mockImplementationOnce(async () => { throw new Error("queue exploded"); }); @@ -210,7 +213,7 @@ describe("InputController orphaned submit", () => { expect(spies.showError).toHaveBeenCalledWith("queue exploded"); // The message survives the failure: text and images return to the editor. expect(editor.getText()).toBe("doomed message"); - expect(ctx.pendingImages).toEqual([image]); + expect(ctx.editor.pendingImages).toEqual([image]); // The signature must not leak for a message that never started. expect(ctx.locallySubmittedUserSignatures.has("doomed message\u00001")).toBe(false); }); @@ -229,7 +232,7 @@ describe("InputController orphaned submit", () => { expect(restored).toBe(1); expect(editor.getText()).toBe("queued with image"); - expect(ctx.pendingImages).toEqual([image]); - expect(ctx.pendingImageLinks).toEqual([undefined]); + expect(ctx.editor.pendingImages).toEqual([image]); + expect(ctx.editor.pendingImageLinks).toEqual([undefined]); }); }); diff --git a/packages/coding-agent/test/input-controller-python-prefix.test.ts b/packages/coding-agent/test/input-controller-python-prefix.test.ts index f57239f65..92cd133ca 100644 --- a/packages/coding-agent/test/input-controller-python-prefix.test.ts +++ b/packages/coding-agent/test/input-controller-python-prefix.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "bun:test"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; @@ -11,6 +12,8 @@ type FakeEditor = { setActionKeys(action: string, keys: string[]): void; setCustomKeyHandler(key: string, handler: () => void): void; clearCustomKeyHandlers(): void; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; }; function createContext() { @@ -33,6 +36,8 @@ function createContext() { setActionKeys: vi.fn(), setCustomKeyHandler: vi.fn(), clearCustomKeyHandlers: vi.fn(), + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], }; const ctx = { @@ -49,8 +54,6 @@ function createContext() { getQueuedMessages: () => ({ steering: [], followUp: [] }), } as unknown as InteractiveModeContext["session"], sessionManager: { getSessionName: () => "named-session" } as unknown as InteractiveModeContext["sessionManager"], - pendingImages: [] as InteractiveModeContext["pendingImages"], - pendingImageLinks: [] as InteractiveModeContext["pendingImageLinks"], compactionQueuedMessages: [] as InteractiveModeContext["compactionQueuedMessages"], locallySubmittedUserSignatures: new Set(), onInputCallback, diff --git a/packages/coding-agent/test/input-controller-skill-queue.test.ts b/packages/coding-agent/test/input-controller-skill-queue.test.ts index 51ce332a9..c8a89339a 100644 --- a/packages/coding-agent/test/input-controller-skill-queue.test.ts +++ b/packages/coding-agent/test/input-controller-skill-queue.test.ts @@ -8,6 +8,7 @@ import { afterEach, beforeEach, describe, expect, it, type Mock, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; @@ -28,6 +29,8 @@ type StubEditor = { getText: () => string; addToHistory: Mock<(...args: unknown[]) => unknown>; onSubmit?: (text: string) => Promise; + pendingImages: ImageContent[]; + pendingImageLinks: (string | undefined)[]; }; type PromptCustomMessage = Mock< @@ -53,6 +56,8 @@ function createStubInputControllerContext(opts: { skillCommands: Map {}); const prompt = vi.fn(async (_text: string, _options?: unknown) => {}); @@ -83,8 +88,6 @@ function createStubInputControllerContext(opts: { skillCommands: Map(), @@ -448,6 +451,8 @@ function createStubInteractiveModeContextForUiHelpers(session: AgentSession) { return editorText; }, addToHistory: vi.fn(), + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], }; const pendingMessagesContainer = new Container(); const requestRender = vi.fn(); diff --git a/packages/coding-agent/test/issue-2375-repro.test.ts b/packages/coding-agent/test/issue-2375-repro.test.ts index f4e8f834a..a6631819c 100644 --- a/packages/coding-agent/test/issue-2375-repro.test.ts +++ b/packages/coding-agent/test/issue-2375-repro.test.ts @@ -17,6 +17,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as os from "node:os"; import * as path from "node:path"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; @@ -41,14 +42,18 @@ function createContext() { const requestRender = vi.fn(); const showStatus = vi.fn(); const ctx = { - editor: { pasteText, insertText, imageLinks: undefined } as unknown as InteractiveModeContext["editor"], + editor: { + pasteText, + insertText, + imageLinks: undefined, + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], + } as unknown as InteractiveModeContext["editor"], ui: { requestRender, getFocused: () => null } as unknown as InteractiveModeContext["ui"], sessionManager: { getCwd: () => process.cwd(), putBlob: async () => ({ hash: "h", path: "/tmp/h.png", displayPath: "/tmp/h.png" }), } as unknown as InteractiveModeContext["sessionManager"], - pendingImages: [] as InteractiveModeContext["pendingImages"], - pendingImageLinks: [] as InteractiveModeContext["pendingImageLinks"], showStatus, } as unknown as InteractiveModeContext; return { ctx, spies: { pasteText, insertText, requestRender, showStatus } }; @@ -150,8 +155,8 @@ describe("InputController.handleImagePathPaste (issue #2375)", () => { expect(spies.pasteText).not.toHaveBeenCalled(); expect(spies.showStatus).not.toHaveBeenCalled(); - expect(ctx.pendingImages.length).toBe(1); - expect(ctx.pendingImages[0]?.mimeType).toBe("image/png"); + expect(ctx.editor.pendingImages.length).toBe(1); + expect(ctx.editor.pendingImages[0]?.mimeType).toBe("image/png"); }); it("locally: attaches the clipboard image when the pasted path resolves to a non-image file", async () => { @@ -174,7 +179,7 @@ describe("InputController.handleImagePathPaste (issue #2375)", () => { } expect(spies.pasteText).not.toHaveBeenCalled(); - expect(ctx.pendingImages.length).toBe(1); - expect(ctx.pendingImages[0]?.mimeType).toBe("image/png"); + expect(ctx.editor.pendingImages.length).toBe(1); + expect(ctx.editor.pendingImages[0]?.mimeType).toBe("image/png"); }); }); diff --git a/packages/coding-agent/test/issue-interrupt-and-flush-empty-messages.test.ts b/packages/coding-agent/test/issue-interrupt-and-flush-empty-messages.test.ts index 51499fd19..8ae8e55e2 100644 --- a/packages/coding-agent/test/issue-interrupt-and-flush-empty-messages.test.ts +++ b/packages/coding-agent/test/issue-interrupt-and-flush-empty-messages.test.ts @@ -1,4 +1,5 @@ import { describe, expect, it, vi } from "bun:test"; +import type { ImageContent } from "@oh-my-pi/pi-ai"; import { InputController } from "@oh-my-pi/pi-coding-agent/modes/controllers/input-controller"; import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; import { USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-agent/session/messages"; @@ -19,6 +20,8 @@ function createContext() { return editorText; }, addToHistory: vi.fn(), + pendingImages: [] as ImageContent[], + pendingImageLinks: [] as (string | undefined)[], }, ui: { requestRender }, session: { @@ -34,8 +37,6 @@ function createContext() { get viewSession() { return (this as typeof ctx).session; }, - pendingImages: [], - pendingImageLinks: [], compactionQueuedMessages: [], locallySubmittedUserSignatures: new Set(), isBashMode: false,