fix(tui): keep pasted images when a mode command submits the draft
`/goal <objective>`, `/plan <prompt>` and `/vibe <prompt>` promote the
composer draft into the first turn, but built their submission from the
draft *text* only:
this.onInputCallback(this.startPendingSubmission({ text: objective }));
The editor-submit path in `InputController` passes
`editor.pendingImages`/`pendingImageLinks` alongside the text; these four
call sites did not. A draft holding pasted screenshots therefore reached
the model with its positional `[Image #N, WxH]` markers intact and every
image payload missing, so the agent saw markers pointing at nothing and
`read "Image #1"` resolved against an empty list.
The payload was not only dropped, it also outlived the draft: the mode
commands cleared the composer with `editor.setText("")`, which leaves
`pendingImages` attached. The orphans then rode along with whatever the
user typed next, one index off, which is how a later message can attach a
screenshot the user never re-pasted.
Measured on 259 image-bearing user messages across 10 local session logs
(v17.2.x): 36 of 37 messages submitted as a goal objective lost every
image, against 190 of 198 preserved on the ordinary submit path.
Fix:
- `#takeDraftImages()` detaches the composer's pending images and links,
and all four mode-command submissions spread it into
`startPendingSubmission` (`cancelPendingSubmission` already restores
them when a submission is cancelled).
- `/goal`, `/guided-goal`, `/plan` and `/vibe` clear the draft with
`editor.clearDraft()` instead of `editor.setText("")`, so images can
never outlive the text they were pasted into (the streaming branch of
`/goal` never submits, so its draft must die whole).
Tests: two regression cases in `goal-mode-integration.test.ts` assert the
objective submission carries the image and empties the composer; both
fail on the previous behaviour with `images: undefined`. The `/plan`,
`/goal` and `/guided-goal` slash stubs now model `clearDraft`.
This commit is contained in:
@@ -1584,6 +1584,22 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
this.addMessageToChat(message, options);
|
||||
}
|
||||
|
||||
/** Detach the composer's pending images so they ride along with the submission
|
||||
* that consumes the draft. Mode commands that promote the draft into a turn
|
||||
* (`/goal`, `/plan`, `/vibe`) build their submission from the draft *text*
|
||||
* only; without this the positional `[Image #N]` markers survive into the
|
||||
* message while every payload is dropped, and the orphaned images leak into
|
||||
* whatever the user sends next. Mirrors the editor-submit path in
|
||||
* `InputController`. `cancelPendingSubmission` restores them on cancel. */
|
||||
#takeDraftImages(): { images?: ImageContent[]; imageLinks?: (string | undefined)[] } {
|
||||
if (this.editor.pendingImages.length === 0) return {};
|
||||
const images = [...this.editor.pendingImages];
|
||||
const imageLinks = this.editor.pendingImageLinks.length > 0 ? [...this.editor.pendingImageLinks] : undefined;
|
||||
this.editor.pendingImages = [];
|
||||
this.editor.pendingImageLinks = [];
|
||||
return { images, imageLinks };
|
||||
}
|
||||
|
||||
startPendingSubmission(input: {
|
||||
text: string;
|
||||
images?: ImageContent[];
|
||||
@@ -3321,7 +3337,7 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
}
|
||||
await this.#enterPlanMode();
|
||||
if (initialPrompt && this.onInputCallback) {
|
||||
this.onInputCallback(this.startPendingSubmission({ text: initialPrompt }));
|
||||
this.onInputCallback(this.startPendingSubmission({ text: initialPrompt, ...this.#takeDraftImages() }));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3347,7 +3363,7 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
}
|
||||
await this.#enterVibeMode();
|
||||
if (initialPrompt && this.onInputCallback) {
|
||||
this.onInputCallback(this.startPendingSubmission({ text: initialPrompt }));
|
||||
this.onInputCallback(this.startPendingSubmission({ text: initialPrompt, ...this.#takeDraftImages() }));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3669,7 +3685,7 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
await this.#enterGoalMode({ objective, silent: true });
|
||||
this.#resetGoalContinuationSuppression();
|
||||
if (!this.session.isStreaming && this.onInputCallback) {
|
||||
this.onInputCallback(this.startPendingSubmission({ text: objective }));
|
||||
this.onInputCallback(this.startPendingSubmission({ text: objective, ...this.#takeDraftImages() }));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -3684,7 +3700,7 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
await this.session.sendGoalModeContext({ deliverAs: "steer" });
|
||||
}
|
||||
if (!this.session.isStreaming && this.onInputCallback) {
|
||||
this.onInputCallback(this.startPendingSubmission({ text: objective }));
|
||||
this.onInputCallback(this.startPendingSubmission({ text: objective, ...this.#takeDraftImages() }));
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -183,7 +183,7 @@ export const BUILTIN_MODE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec> = [
|
||||
},
|
||||
handleTui: async (command, runtime) => {
|
||||
await runtime.ctx.handlePlanModeCommand(command.args || undefined);
|
||||
runtime.ctx.editor.setText("");
|
||||
runtime.ctx.editor.clearDraft();
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -209,7 +209,7 @@ export const BUILTIN_MODE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec> = [
|
||||
},
|
||||
handleTui: async (command, runtime) => {
|
||||
await runtime.ctx.handleVibeModeCommand(command.args || undefined);
|
||||
runtime.ctx.editor.setText("");
|
||||
runtime.ctx.editor.clearDraft();
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -233,7 +233,7 @@ export const BUILTIN_MODE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec> = [
|
||||
},
|
||||
handleTui: async (command, runtime) => {
|
||||
await runtime.ctx.handleGoalModeCommand(command.args || undefined);
|
||||
runtime.ctx.editor.setText("");
|
||||
runtime.ctx.editor.clearDraft();
|
||||
},
|
||||
},
|
||||
{
|
||||
@@ -245,7 +245,7 @@ export const BUILTIN_MODE_SLASH_COMMANDS: ReadonlyArray<SlashCommandSpec> = [
|
||||
// Clear the slash draft BEFORE the await: the handler blocks for the
|
||||
// whole kickoff turn, and a post-await clear would wipe an answer the
|
||||
// user starts typing while the first interview question streams.
|
||||
runtime.ctx.editor.setText("");
|
||||
runtime.ctx.editor.clearDraft();
|
||||
await runtime.ctx.handleGuidedGoalCommand(command.args || undefined);
|
||||
},
|
||||
},
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import type { Model } from "@oh-my-pi/pi-ai";
|
||||
import type { ImageContent, Model } from "@oh-my-pi/pi-ai";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { GoalTool } from "@oh-my-pi/pi-coding-agent/goals/tools/goal-tool";
|
||||
@@ -134,15 +134,19 @@ async function waitForMicrotasks(): Promise<void> {
|
||||
async function armInputWaiter(mode: InteractiveMode): Promise<{
|
||||
inputPromise: Promise<void>;
|
||||
getResolvedText: () => string | undefined;
|
||||
getResolvedImages: () => ImageContent[] | undefined;
|
||||
}> {
|
||||
let resolvedText: string | undefined;
|
||||
let resolvedImages: ImageContent[] | undefined;
|
||||
const inputPromise = mode.getUserInput().then(input => {
|
||||
resolvedText = input.text;
|
||||
resolvedImages = input.images;
|
||||
});
|
||||
await waitForMicrotasks();
|
||||
return {
|
||||
inputPromise,
|
||||
getResolvedText: () => resolvedText,
|
||||
getResolvedImages: () => resolvedImages,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -241,6 +245,39 @@ describe("InteractiveMode goal mode integration", () => {
|
||||
await waiter.inputPromise;
|
||||
});
|
||||
|
||||
it("carries composer images into the initial goal objective submission", async () => {
|
||||
// Regression: `/goal <objective>` built its submission from the draft text
|
||||
// alone, so the objective kept its positional `[Image #1]` marker while the
|
||||
// payload stayed behind on the editor and leaked into the next message.
|
||||
const image: ImageContent = { type: "image", data: "aW1hZ2U=", mimeType: "image/png" };
|
||||
harness.mode.editor.pendingImages = [image];
|
||||
harness.mode.editor.pendingImageLinks = ["file:///shot.png"];
|
||||
const waiter = await armInputWaiter(harness.mode);
|
||||
|
||||
await harness.mode.handleGoalModeCommand("[Image #1, 10x10] fix this");
|
||||
await waiter.inputPromise;
|
||||
|
||||
expect(waiter.getResolvedText()).toBe("[Image #1, 10x10] fix this");
|
||||
expect(waiter.getResolvedImages()).toEqual([image]);
|
||||
expect(harness.mode.editor.pendingImages).toEqual([]);
|
||||
expect(harness.mode.editor.pendingImageLinks).toEqual([]);
|
||||
});
|
||||
|
||||
it("carries composer images into the replacement goal objective submission", async () => {
|
||||
await harness.mode.handleGoalModeCommand("Ship the release");
|
||||
const image: ImageContent = { type: "image", data: "cmVwbGFjZQ==", mimeType: "image/png" };
|
||||
harness.mode.editor.pendingImages = [image];
|
||||
harness.mode.editor.pendingImageLinks = [undefined];
|
||||
const waiter = await armInputWaiter(harness.mode);
|
||||
|
||||
await harness.mode.handleGoalModeCommand("set [Image #1, 10x10] replace this");
|
||||
await waiter.inputPromise;
|
||||
|
||||
expect(waiter.getResolvedText()).toBe("[Image #1, 10x10] replace this");
|
||||
expect(waiter.getResolvedImages()).toEqual([image]);
|
||||
expect(harness.mode.editor.pendingImages).toEqual([]);
|
||||
});
|
||||
|
||||
it("includes escaped live todo state in hidden goal context during continuations", async () => {
|
||||
await harness.session.setActiveToolsByName(["read", "todo"]);
|
||||
await harness.mode.handleGoalModeCommand("Ship the release");
|
||||
|
||||
@@ -4,13 +4,13 @@ import { executeBuiltinSlashCommand } from "@oh-my-pi/pi-coding-agent/slash-comm
|
||||
|
||||
function createRuntime(handler: () => Promise<void>) {
|
||||
const handleGuidedGoalCommand = vi.fn(handler);
|
||||
const setText = vi.fn();
|
||||
const clearDraft = vi.fn();
|
||||
return {
|
||||
handleGuidedGoalCommand,
|
||||
setText,
|
||||
clearDraft,
|
||||
runtime: {
|
||||
ctx: {
|
||||
editor: { setText } as unknown as InteractiveModeContext["editor"],
|
||||
editor: { clearDraft } as unknown as InteractiveModeContext["editor"],
|
||||
handleGuidedGoalCommand,
|
||||
} as unknown as InteractiveModeContext,
|
||||
},
|
||||
@@ -29,12 +29,12 @@ describe("/guided-goal slash command", () => {
|
||||
|
||||
// The command text must be gone before the turn resolves, so an answer
|
||||
// typed while the first question streams is never wiped.
|
||||
expect(harness.setText).toHaveBeenCalledWith("");
|
||||
harness.setText.mockClear();
|
||||
expect(harness.clearDraft).toHaveBeenCalled();
|
||||
harness.clearDraft.mockClear();
|
||||
|
||||
resolve();
|
||||
expect(await dispatched).toBe(true);
|
||||
expect(harness.setText).not.toHaveBeenCalled();
|
||||
expect(harness.clearDraft).not.toHaveBeenCalled();
|
||||
expect(harness.handleGuidedGoalCommand).toHaveBeenCalledWith("ship the release");
|
||||
});
|
||||
|
||||
|
||||
@@ -12,10 +12,10 @@ import { executeBuiltinSlashCommand } from "@oh-my-pi/pi-coding-agent/slash-comm
|
||||
function createPlanHarness(opts: { planModeEnabled: boolean; confirmExit: boolean }) {
|
||||
const state = { planModeEnabled: opts.planModeEnabled };
|
||||
const addToHistory = mock((_text: string) => {});
|
||||
const setText = mock((_text: string) => {});
|
||||
const clearDraft = mock((_historyText?: string) => {});
|
||||
|
||||
const ctx = {
|
||||
editor: { addToHistory, setText } as unknown as InteractiveModeContext["editor"],
|
||||
editor: { addToHistory, clearDraft } as unknown as InteractiveModeContext["editor"],
|
||||
get planModeEnabled() {
|
||||
return state.planModeEnabled;
|
||||
},
|
||||
@@ -33,17 +33,17 @@ function createPlanHarness(opts: { planModeEnabled: boolean; confirmExit: boolea
|
||||
runtime: { ctx },
|
||||
state,
|
||||
addToHistory,
|
||||
setText,
|
||||
clearDraft,
|
||||
};
|
||||
}
|
||||
|
||||
function createGoalHarness(opts: { goalModeEnabled: boolean; dropOnCall: boolean }) {
|
||||
const state = { goalModeEnabled: opts.goalModeEnabled };
|
||||
const addToHistory = mock((_text: string) => {});
|
||||
const setText = mock((_text: string) => {});
|
||||
const clearDraft = mock((_historyText?: string) => {});
|
||||
|
||||
const ctx = {
|
||||
editor: { addToHistory, setText } as unknown as InteractiveModeContext["editor"],
|
||||
editor: { addToHistory, clearDraft } as unknown as InteractiveModeContext["editor"],
|
||||
get goalModeEnabled() {
|
||||
return state.goalModeEnabled;
|
||||
},
|
||||
@@ -57,7 +57,7 @@ function createGoalHarness(opts: { goalModeEnabled: boolean; dropOnCall: boolean
|
||||
runtime: { ctx },
|
||||
state,
|
||||
addToHistory,
|
||||
setText,
|
||||
clearDraft,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -70,7 +70,7 @@ describe("/plan handler when already active", () => {
|
||||
expect(handled).toBe(true);
|
||||
// Sanity check: exit was confirmed, so plan mode is now off.
|
||||
expect(h.state.planModeEnabled).toBe(false);
|
||||
expect(h.setText).toHaveBeenCalledWith("");
|
||||
expect(h.clearDraft).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("keeps plan mode active when user cancels exit", async () => {
|
||||
@@ -81,7 +81,7 @@ describe("/plan handler when already active", () => {
|
||||
expect(handled).toBe(true);
|
||||
// Cancel: plan mode stays active.
|
||||
expect(h.state.planModeEnabled).toBe(true);
|
||||
expect(h.setText).toHaveBeenCalledWith("");
|
||||
expect(h.clearDraft).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("enters plan mode when invoked for the first time", async () => {
|
||||
@@ -90,7 +90,7 @@ describe("/plan handler when already active", () => {
|
||||
await executeBuiltinSlashCommand("/plan hello world", h.runtime);
|
||||
|
||||
expect(h.state.planModeEnabled).toBe(true);
|
||||
expect(h.setText).toHaveBeenCalledWith("");
|
||||
expect(h.clearDraft).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
|
||||
|
||||
Reference in New Issue
Block a user