From 2e431c0c212b11cf4c67651bc3c579ad44406a5e Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 14 Jun 2026 00:24:28 +0000 Subject: [PATCH 1/2] fix(coding-agent): renumber image markers when restoring queued messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `restoreQueuedMessagesToEditor` prepended queued text but appended queued images to `pendingImages`. Positional `[Image #N]` lookup at submit time therefore broke whenever the editor draft already held pending image(s): queued markers (numbered 1..K against their own image list) collided with draft markers (1..M) and resolved to the wrong images; queued images landing past slot M were orphaned. Add `shiftImageMarkers(text, offset)` to `image-references.ts` and have `restoreQueuedMessagesToEditor` walk each queued message in order, shifting its markers by the running pending-image count (existing draft images plus images already pulled in from earlier queued messages). Draft markers stay untouched because draft images keep their original slots. Paste markers are left alone — those are owned by the editor's paste store, not the pending-image buffer, and queued message text never carries unmaterialized `[Paste #N]` because the editor expands paste markers in `getExpandedText()` before `onSubmit` fires. Regression test seeds a draft image + a queued image-message in `input-controller-compaction-image.test.ts` (per acceptance) and locks the marker -> image mapping after restore. Unit tests for `shiftImageMarkers` cover the WxH tail, Paste-marker passthrough, and the zero-offset no-op. Fixes #2531 --- packages/coding-agent/CHANGELOG.md | 1 + .../src/modes/controllers/input-controller.ts | 28 ++++- .../src/modes/image-references.ts | 17 +++ .../input-controller-compaction-image.test.ts | 113 ++++++++++++++++++ .../test/modes/image-references.test.ts | 17 ++- 5 files changed, 171 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6f1d1b374..61c967ba1 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -5,6 +5,7 @@ ### Fixed - Fixed session JSONL persistence so the first assistant turn materializes the file synchronously, leaves the append writer open, and writes later entries with a sync append writer even during writer-close races instead of waiting on a queued rewrite. +- Fixed `restoreQueuedMessagesToEditor` (Alt+Up dequeue and Esc-abort) producing colliding `[Image #N]` markers when the editor draft already held pending image(s): queued text was prepended but queued images were appended, so positional marker → image lookup at submit time resolved to the wrong image. Each queued message's image markers are now renumbered by the running pending-image count before merge so the combined text stays aligned with the merged `pendingImages` order ([#2531](https://github.com/can1357/oh-my-pi/issues/2531)). ## [15.12.5] - 2026-06-13 ### Changed diff --git a/packages/coding-agent/src/modes/controllers/input-controller.ts b/packages/coding-agent/src/modes/controllers/input-controller.ts index e56b34cc5..ef4a45deb 100644 --- a/packages/coding-agent/src/modes/controllers/input-controller.ts +++ b/packages/coding-agent/src/modes/controllers/input-controller.ts @@ -7,7 +7,7 @@ import { AssistantMessageComponent } from "../../modes/components/assistant-mess import { renderSegmentTrack } from "../../modes/components/segment-track"; import { TinyTitleDownloadProgressComponent } from "../../modes/components/tiny-title-download-progress"; import { expandEmoticons } from "../../modes/emoji-autocomplete"; -import { materializeImageReferenceLinks } from "../../modes/image-references"; +import { materializeImageReferenceLinks, shiftImageMarkers } from "../../modes/image-references"; import { createPromptActionAutocompleteProvider } from "../../modes/prompt-action-autocomplete"; import type { InteractiveModeContext } from "../../modes/types"; import manualContinuePrompt from "../../prompts/system/manual-continue.md" with { type: "text" }; @@ -932,14 +932,34 @@ export class InputController { } return 0; } - const queuedText = allQueued.map(e => e.text).join("\n\n"); + // Image markers are positional: `[Image #N]` ↔ `pendingImages[N-1]`. Each + // queued message numbered its markers against its own local image list + // (1..K). Because we prepend the queued text but append the queued images + // to `pendingImages`, any existing draft images (M of them) — plus images + // already pulled in by earlier queued messages — shift the slot index that + // every marker must point to. Bumping each message's markers by the + // running offset keeps the merged text aligned with the merged + // `pendingImages` order; draft markers stay valid because draft images + // keep their original positions. + const queuedImages = allQueued.flatMap(e => e.images ?? []); + let queuedText: string; + if (queuedImages.length > 0) { + const parts: string[] = []; + let imageOffset = this.ctx.pendingImages.length; + for (const entry of allQueued) { + parts.push(shiftImageMarkers(entry.text, imageOffset)); + if (entry.images && entry.images.length > 0) imageOffset += entry.images.length; + } + queuedText = parts.join("\n\n"); + } else { + queuedText = allQueued.map(e => e.text).join("\n\n"); + } const currentText = options?.currentText ?? this.ctx.editor.getText(); const combinedText = [queuedText, currentText].filter(t => t.trim()).join("\n\n"); this.ctx.editor.setText(combinedText); // Hand queued images back to the pending-image buffer (links are // re-materialized lazily; the restored text already carries the - // `[Image #N, WxH]` markers). - const queuedImages = allQueued.flatMap(e => e.images ?? []); + // renumbered `[Image #N, WxH]` markers). if (queuedImages.length > 0) { this.ctx.pendingImages.push(...queuedImages); this.ctx.pendingImageLinks.push(...queuedImages.map(() => undefined)); diff --git a/packages/coding-agent/src/modes/image-references.ts b/packages/coding-agent/src/modes/image-references.ts index 9dae460cf..b5ab077f0 100644 --- a/packages/coding-agent/src/modes/image-references.ts +++ b/packages/coding-agent/src/modes/image-references.ts @@ -8,6 +8,23 @@ import { fileHyperlink } from "../tui/hyperlink"; * tail (`, …`) is captured loosely (no `]`/newline) so future label tweaks keep matching. */ export const PLACEHOLDER_REGEX = /\[(Image|Paste) #([1-9]\d*)(?:,[^\]\n]*)?\]/g; +/** Matches a single `[Image #N]` / `[Image #N, WxH]` marker. Group 1 is the + * 1-based index, group 2 the optional metadata tail (leading comma, no `]` or + * newline) so future label tweaks keep matching. Paste markers are excluded + * on purpose: their numbering is owned by the editor's paste store, not by + * the pending-image buffer. */ +const IMAGE_MARKER_REGEX = /\[Image #([1-9]\d*)((?:,[^\]\n]*)?)\]/g; + +/** Renumber every `[Image #N]` marker in `text` by `offset` (added to the + * existing index), preserving the optional `, WxH` tail. Paste markers are + * left untouched. Used when restoring queued image-messages back into a draft + * that already holds pending images so the merged text's positional markers + * still line up with `pendingImages`. */ +export function shiftImageMarkers(text: string, offset: number): string { + if (offset === 0) return text; + return text.replace(IMAGE_MARKER_REGEX, (_match, idx: string, tail: string) => `[Image #${Number(idx) + offset}${tail}]`); +} + type ImageBlobWriter = (data: Buffer, options?: { extension?: string }) => Promise; type ImageBlobWriterSync = (data: Buffer, options?: { extension?: string }) => BlobPutResult; 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 14526b109..6ef7bb454 100644 --- a/packages/coding-agent/test/input-controller-compaction-image.test.ts +++ b/packages/coding-agent/test/input-controller-compaction-image.test.ts @@ -13,10 +13,16 @@ * - On flush, the first queued prompt forwards its images via `session.prompt`. * - On a `willRetry` flush, a queued follow-up forwards its images via * `session.followUp` (the `#deliverQueuedMessage` path). + * - When `restoreQueuedMessagesToEditor` reinjects queued image-messages + * into a draft that already holds pending image(s), the merged text's + * `[Image #N]` markers stay aligned with the merged `pendingImages` order + * (#2531). Bug-by-design before that fix: queued markers (1..K) collided + * with the draft's leading markers and submit picked the wrong images. */ import { beforeAll, describe, expect, mock, test } 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 { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; import type { CompactionQueuedMessage, InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; import { UiHelpers } from "@oh-my-pi/pi-coding-agent/modes/utils/ui-helpers"; @@ -127,3 +133,110 @@ describe("compaction queue image forwarding", () => { expect(followUpCalls).toEqual([{ text: "and this one", images: [image] }]); }); }); + +/** + * Restore path: when the editor draft already holds pending image(s) and a + * queued image-message is restored (Alt+Up, Esc-abort, …), the merged text's + * `[Image #N]` markers must still map positionally to `pendingImages`. The + * old code prepended queued text but appended queued images, so the queued + * markers (1..K) collided with the draft markers (1..M) and resolved to the + * wrong images at submit time. + */ +describe("restoreQueuedMessagesToEditor image marker alignment", () => { + function makeRestoreCtx(opts: { + draftText?: string; + draftImages?: ImageContent[]; + queued?: { text: string; images?: ImageContent[] }[]; + }) { + let editorText = opts.draftText ?? ""; + const editor = { + setText: (text: string) => { + editorText = text; + }, + getText: () => editorText, + addToHistory: () => {}, + imageLinks: undefined as (string | undefined)[] | undefined, + }; + const session = { + clearQueue: mock(() => ({ steering: opts.queued ?? [], followUp: [] })), + abort: mock(async () => {}), + }; + const ctx = { + session, + editor, + pendingImages: opts.draftImages ? [...opts.draftImages] : ([] as ImageContent[]), + pendingImageLinks: opts.draftImages ? opts.draftImages.map(() => undefined) : ([] as (string | undefined)[]), + locallySubmittedUserSignatures: new Set(), + updatePendingMessagesDisplay: () => {}, + } as unknown as InteractiveModeContext; + return { ctx, editor }; + } + + test("renumbers queued markers when the draft already holds a pending image", () => { + const draftImg = img("ZHJhZnQ="); + const queuedImg = img("cXVldWVk"); + const { ctx, editor } = makeRestoreCtx({ + draftText: "[Image #1] draft text", + draftImages: [draftImg], + queued: [{ text: "[Image #1] queued text", images: [queuedImg] }], + }); + + const restored = new InputController(ctx).restoreQueuedMessagesToEditor(); + + // The draft marker stays at #1 (its image kept slot 0); the queued + // 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]); + // Marker → image positional mapping after restore. + expect(ctx.pendingImages[0]).toBe(draftImg); // matches [Image #1] + expect(ctx.pendingImages[1]).toBe(queuedImg); // matches [Image #2] + }); + + test("preserves the WxH metadata tail when renumbering", () => { + const draftImg = img("ZHJhZnQ="); + const queuedImg = img("cXVldWVk"); + const { ctx, editor } = makeRestoreCtx({ + draftText: "[Image #1, 100x100]", + draftImages: [draftImg], + queued: [{ text: "look [Image #1, 800x600] now", images: [queuedImg] }], + }); + + new InputController(ctx).restoreQueuedMessagesToEditor(); + + expect(editor.getText()).toBe("look [Image #2, 800x600] now\n\n[Image #1, 100x100]"); + }); + + test("accumulates the offset across multiple queued image-messages", () => { + const draftImg = img("ZHJhZnQ="); + const queued1 = img("cTE="); + const queued2a = img("cTJh"); + const queued2b = img("cTJi"); + const { ctx, editor } = makeRestoreCtx({ + draftText: "see [Image #1]", + draftImages: [draftImg], + queued: [ + { text: "first [Image #1]", images: [queued1] }, + { text: "second [Image #1] and [Image #2]", images: [queued2a, queued2b] }, + ], + }); + + new InputController(ctx).restoreQueuedMessagesToEditor(); + + // 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]); + }); + + test("leaves the queued text untouched when the draft has no pending images", () => { + const queuedImg = img("cXVldWVk"); + const { ctx, editor } = makeRestoreCtx({ + queued: [{ text: "[Image #1] queued", images: [queuedImg] }], + }); + + new InputController(ctx).restoreQueuedMessagesToEditor(); + + expect(editor.getText()).toBe("[Image #1] queued"); + expect(ctx.pendingImages).toEqual([queuedImg]); + }); +}); diff --git a/packages/coding-agent/test/modes/image-references.test.ts b/packages/coding-agent/test/modes/image-references.test.ts index 68a406f6c..7b2b281cd 100644 --- a/packages/coding-agent/test/modes/image-references.test.ts +++ b/packages/coding-agent/test/modes/image-references.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from "bun:test"; -import { type PlaceholderKind, renderPlaceholders } from "@oh-my-pi/pi-coding-agent/modes/image-references"; +import { type PlaceholderKind, renderPlaceholders, shiftImageMarkers } from "@oh-my-pi/pi-coding-agent/modes/image-references"; function capture(text: string): { out: string; @@ -43,3 +43,18 @@ describe("renderPlaceholders", () => { expect(refs).toHaveLength(0); }); }); + +describe("shiftImageMarkers", () => { + it("returns text unchanged when the offset is zero", () => { + const text = "[Image #1] then [Image #2, 100x100] and [Paste #3, +5 lines]"; + expect(shiftImageMarkers(text, 0)).toBe(text); + }); + + it("renumbers every Image marker by the offset and preserves the WxH tail", () => { + expect(shiftImageMarkers("see [Image #1, 800x600] then [Image #2]", 3)).toBe("see [Image #4, 800x600] then [Image #5]"); + }); + + it("never touches Paste markers", () => { + expect(shiftImageMarkers("[Image #1] [Paste #1, +5 lines]", 2)).toBe("[Image #3] [Paste #1, +5 lines]"); + }); +}); From f41007d7daa40836eaba17729278c4b01fa424ef Mon Sep 17 00:00:00 2001 From: roboomp Date: Sun, 14 Jun 2026 00:24:34 +0000 Subject: [PATCH 2/2] style: bun run fix --- packages/coding-agent/src/modes/image-references.ts | 5 ++++- .../coding-agent/test/modes/image-references.test.ts | 10 ++++++++-- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/packages/coding-agent/src/modes/image-references.ts b/packages/coding-agent/src/modes/image-references.ts index b5ab077f0..754c14ee9 100644 --- a/packages/coding-agent/src/modes/image-references.ts +++ b/packages/coding-agent/src/modes/image-references.ts @@ -22,7 +22,10 @@ const IMAGE_MARKER_REGEX = /\[Image #([1-9]\d*)((?:,[^\]\n]*)?)\]/g; * still line up with `pendingImages`. */ export function shiftImageMarkers(text: string, offset: number): string { if (offset === 0) return text; - return text.replace(IMAGE_MARKER_REGEX, (_match, idx: string, tail: string) => `[Image #${Number(idx) + offset}${tail}]`); + return text.replace( + IMAGE_MARKER_REGEX, + (_match, idx: string, tail: string) => `[Image #${Number(idx) + offset}${tail}]`, + ); } type ImageBlobWriter = (data: Buffer, options?: { extension?: string }) => Promise; diff --git a/packages/coding-agent/test/modes/image-references.test.ts b/packages/coding-agent/test/modes/image-references.test.ts index 7b2b281cd..8952eab07 100644 --- a/packages/coding-agent/test/modes/image-references.test.ts +++ b/packages/coding-agent/test/modes/image-references.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "bun:test"; -import { type PlaceholderKind, renderPlaceholders, shiftImageMarkers } from "@oh-my-pi/pi-coding-agent/modes/image-references"; +import { + type PlaceholderKind, + renderPlaceholders, + shiftImageMarkers, +} from "@oh-my-pi/pi-coding-agent/modes/image-references"; function capture(text: string): { out: string; @@ -51,7 +55,9 @@ describe("shiftImageMarkers", () => { }); it("renumbers every Image marker by the offset and preserves the WxH tail", () => { - expect(shiftImageMarkers("see [Image #1, 800x600] then [Image #2]", 3)).toBe("see [Image #4, 800x600] then [Image #5]"); + expect(shiftImageMarkers("see [Image #1, 800x600] then [Image #2]", 3)).toBe( + "see [Image #4, 800x600] then [Image #5]", + ); }); it("never touches Paste markers", () => {