From 7e5a2be3ee1f85cb0f08de5a62d3c3ee6fb5ecbf Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 27 Jun 2026 00:57:05 +0000 Subject: [PATCH] fix(coding-agent): serialize keystrokes behind in-flight paste so trailing Enter cannot race image attach Address PR #3602 review feedback from chatgpt-codex-connector: when a stdin read carries the empty bracketed paste followed by a trailing keystroke (a user pressing Enter right after Cmd+V), the pre-fix paste path was fire-and-forget. The trailing byte processed synchronously while the clipboard image read was still pending, so submit ran against an empty pendingImages and the image landed on the next draft instead. CustomEditor now tracks in-flight pastes with #pasteInFlight and buffers subsequent input into #pendingInput. #trackAsyncPaste increments the counter, awaits the paste promise, decrements, and drains the queue through handleInput (so requeueing still works if a drained chunk triggers another async paste). For an assembled paste whose remaining bytes are present in the same call, those bytes are pushed onto #pendingInput before the async paste starts, so they always run AFTER it settles. The text-paste branch stays sync and drains its own queue inline. New repro test asserts the call ordering: paste:start fires, the queued Enter does NOT, and only after the paste promise settles does Enter dispatch. --- packages/coding-agent/CHANGELOG.md | 17 +++++ .../src/modes/components/custom-editor.ts | 65 +++++++++++++++---- .../test/issue-3601-repro.test.ts | 38 +++++++++++ 3 files changed, 109 insertions(+), 11 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 8245b5e62..ba219ffa5 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -47,6 +47,23 @@ - Improved the persistent Todo HUD styling and progress indicators to make it self-describing and visually distinct. - Fixed browser screenshots reporting `0x0` dimensions when image headers expose real dimensions. - Fixed `snapcompact` compaction silently falling back to LLM summaries when local preflight rejects the archive. +- Fixed `snapcompact` compaction silently falling back to an LLM summary when local preflight rejects the archive; manual and auto snapcompact now fail locally with the blocker instead of making provider calls. ([#3599](https://github.com/can1357/oh-my-pi/issues/3599)) +- Fixed garbled casing in auto-generated session titles. `normalizeGeneratedTitle` (`packages/coding-agent/src/tiny/text.ts`) used to force Title Case via a `\b\p{Ll}` regex, capitalizing function words ("for" → "For") and amplifying stray model capitals ("dAemon" → "DAemon"). It now reconciles each title token against the user's own message: tokens typed verbatim are kept; proper nouns the user cased distinctively are restored when the model flattened them ("tinyvmm" → "TinyVMM"); lowercase words carrying a stray interior capital the user never wrote are flattened ("dAemon" → "daemon"); and model-cased PascalCase proper nouns ("GitHub", "OAuth") are left untouched. Restoration is limited to distinctively cased source tokens so a message that merely starts with "For" can't force a mid-title "for" to "For". Applies to both the local tiny-model and online pi/smol title paths. +- Fixed IRC broadcasts (`to: "all"`) rendering twice in the main agent's transcript. A subagent broadcast fans out one `bus.send` per live peer, and `listVisibleTo` always includes `Main`, so the main agent received the body once as its own `irc:incoming` card *and* once per other recipient as an `irc:relay` observation of the sibling legs (`Sender → Other`) — identical text shown N+1 times. `IrcTool.#executeSend` now sets `suppressRelay` on every broadcast leg when `Main` is among the targets (it already has the body via its direct incoming card), and `IrcBus.send` skips `#relayToMainUi` for suppressed legs. Direct sub→sub relays, direct messages to `Main`, and `Main`'s own outbound sends are unaffected. +- Fixed Claude API refusals polluting the replayed session context and causing later prompts to refuse again. ([#3592](https://github.com/can1357/oh-my-pi/issues/3592)) +- Fixed browser screenshots reporting `0x0` dimensions when `Bun.Image` rejects an image whose PNG/JPEG header still exposes real dimensions. ([#3577](https://github.com/can1357/oh-my-pi/issues/3577)) +- Fixed Anthropic classifier refusals being persisted as assistant dialogue after no fallback handled them; refusal stops are now displayed as errors but pruned from active and saved context before the next prompt. ([#3591](https://github.com/can1357/oh-my-pi/issues/3591)) +- Fixed the `eval` `tool.*` bridge leaking the harness-internal `i` ("intent") field into MCP `tools/call` requests, so strict-schema servers (Linear, anything with `additionalProperties:false` / Zod `.strict()`) rejected every call with `-32602 unrecognized_keys: ["i"]` while the same call via the direct model tool-call path succeeded. `MCPTool.execute` / `DeferredMCPTool.execute` (`packages/coding-agent/src/mcp/tool-bridge.ts`) now strip `INTENT_FIELD` at the MCP boundary, so an MCP call behaves identically whether issued by the model directly or via the eval `tool.*` bridge; servers that legitimately declare `i` as a real parameter keep it untouched. ([#3575](https://github.com/can1357/oh-my-pi/issues/3575)) +- Fixed the advisor entering a spam loop in which it emitted hundreds of repeated `Stop.`, `Done.`, and `No issue; continue.` `` injections, polluting the primary transcript and destabilizing the watched agent after the task was already complete. The advisor system prompt's rules ("at most one `advise` per update", "NEVER send the same advice twice") are now enforced in code by a new `AdvisorEmissionGuard` on the `enqueueAdvice` boundary in `AgentSession`: it normalizes each note (case-insensitive, punctuation-folded), drops content-free self-talk filler (`stop`/`done`/`no issue continue`/`lgtm`/etc.), dedupes by exact normalized text across the session (bounded FIFO history), and rate-limits to one accepted note per advisor model prompt cycle. Reset on advisor reset (compaction, session switch, `/new`) so a re-primed reviewer can re-raise old issues. ([#3520](https://github.com/can1357/oh-my-pi/issues/3520)) +- Fixed auto-compaction thrashing on a session whose single most-recent kept turn already exceeds the compaction threshold. `prepareCompaction` keeps that turn verbatim (`findCutPoint` never cuts at tool results), so the rewritten context stays above threshold; the context-full / snapcompact success tail scheduled the agent-authored auto-continue (and the overflow/incomplete retry) unconditionally, so the next `agent_end` re-entered `#checkCompaction` over the same oversized tail and re-fired forever. This is the residual loop left after #3247 capped snapcompact's own frame projection — once the frame cap drops below one frame, snapcompact is skipped and the context-full summarizer path still made no headroom. `#runAutoCompaction` now gates the threshold auto-continue on a post-maintenance headroom check (`#compactionCreatedHeadroom`, sharing shake's `COMPACTION_RECOVERY_BAND` hysteresis from #2275) and the overflow/incomplete retry on a separate fit check (`#compactionCreatedRetryFit`, measured after the failed turn is dropped) so a recoverable overflow that fits the window still retries; when a pass frees too little for the relevant path it pauses automatic maintenance and emits a single warning instead of looping. The post-turn threshold check also ignores an assistant's stale pre-compaction `usage` so the scheduled auto-continue cannot re-trip on the kept assistant's old high token count. The headroom check now treats any residual at or below the recovery band as progress: the band sits strictly under the compaction threshold, so a stale/tool-output prune that already pushed the trigger sub-band no longer makes a residual that merely holds the line report a false "no progress" and suppress a valid auto-continue. +- Fixed the advisor prompt allowing confident root-cause claims about tool-call arguments absent from its reviewed transcript, so timeout advice now has to cite observed fields instead of inventing mechanisms like `paths[0]` array flattening. ([#3483](https://github.com/can1357/oh-my-pi/issues/3483)) +- Fixed eval `agent()` helper subagents remaining visible as idle/IRC-revivable peers after the helper call returns; one-shot eval subagents now dispose and unregister at completion. ([#3407](https://github.com/can1357/oh-my-pi/pull/3407)) +- Fixed the parent interactive prompt wedging (frozen, 0% CPU) after a subagent's yield-triggered abort. The executor's abort monitor and abort listener each called `session.abort()` fire-and-forget, so `runSubprocess` could resolve and adopt the subagent before its abort cleanup finished clearing in-flight session state. Active-subagent aborts are now deduped through a single cached cleanup promise that the subprocess awaits (bounded) before finalizing. ([#2805](https://github.com/can1357/oh-my-pi/issues/2805)) +- Fixed thinking blocks appearing in the UI when thinking level is "off". Some providers (MiniMax, GLM, DeepSeek) return thinking blocks even with reasoning disabled; thinking blocks are now auto-hidden when the thinking level is "off", regardless of the `hideThinkingBlock` setting. Toggling thinking block visibility while thinking is off shows a status message instead of silently no-op'ing. ([#626](https://github.com/can1357/oh-my-pi/issues/626)) +- Fixed the TUI usage display failing to resolve a used fraction for limits that only populate `remainingFraction` (no `usedFraction`, `used`/`limit`, or `percent`+`used`). The TUI's local `resolveFraction` was missing the inverted-remaining fallback that the shared `resolveUsedFraction` from `@oh-my-pi/pi-ai` already handles — replaced the local copy with the shared function so the TUI and CLI paths resolve fractions identically. +- Fixed long-running SSH command boxes leaving a stale `⏳ SSH: [host]` header above the final `⇄ SSH: [host]` header in terminal scrollback. The SSH renderer now keeps its partial-result chrome on the pending icon/state and opts the block out of stream-commit while `isPartial` holds (via the new `ToolRenderer.provisionalPartialResult` flag honored by `ToolExecutionComponent.isTranscriptBlockCommitStable`), so the stable-prefix ratchet can't promote the partial header to native scrollback only to have the final render strand it above the settled frame ([#3177](https://github.com/can1357/oh-my-pi/issues/3177)). +- Fixed Gemini over-planning runs that emit long chains of thinking headers (`**Refining …**`, `## Examining …`) without ever issuing a tool call. The session now interrupts that stream, discards the partial reasoning turn, injects a hidden tool-call reminder, and continues with the corrective context instead of burning the full budget on planning. +- Fixed `Cmd+V` on macOS silently dropping image-only clipboard pastes (screenshot via `Cmd+Shift+5` "save to clipboard", Chrome image copy, …) — the user had to fall back to `Ctrl+V`. Follow-up to #3506: that fix handled clipboards exposing a file URL or path; the screenshot path leaves only raw image bytes on the pasteboard. macOS terminals (iTerm2, Terminal.app, Warp, Ghostty without OSC 5522, Windows Terminal forwarding, …) intercept `Cmd+V` and read `NSPasteboardTypeString` first; for an image-only clipboard that read returns `""`, so the terminal forwards a complete-but-empty bracketed paste (`\x1b[200~\x1b[201~`). `CustomEditor.handleInput` inserted the empty payload and the keystroke disappeared. `CustomEditor` now runs its own `BracketedPasteHandler` ahead of the inherited handler so the assembled paste payload is routed regardless of whether the start marker, payload, and end marker arrive in one stdin chunk or are fragmented across several (Windows Terminal under load, certain SSH muxes, tmux extended-keys passthrough, …). Strict-zero-length assembled payloads route to the same `onPasteImage` smart reader the `app.clipboard.pasteImage` keybind uses (attaches the clipboard image, or falls back to the #1628 smart text paste / "clipboard is empty" diagnostic); explicit image-file paths route to `onPasteImagePath` (the #3506 path also benefits from split-chunk assembly); everything else hands off to the base editor's `pasteText` so `[Paste #N]` markers, autocomplete, and undo state stay intact. Whitespace-only pastes are preserved as literal text. Trailing keystrokes that arrive in the same stdin read as the paste (a user hitting `Enter` right after `Cmd+V`) are queued behind the in-flight clipboard read and only dispatched once the image has reached `pendingImages`, so submit can't fire against an empty draft and leave the image stranded on the next prompt. ([#3601](https://github.com/can1357/oh-my-pi/issues/3601)) ## [16.1.23] - 2026-06-26 diff --git a/packages/coding-agent/src/modes/components/custom-editor.ts b/packages/coding-agent/src/modes/components/custom-editor.ts index c923798c4..bf144ff11 100644 --- a/packages/coding-agent/src/modes/components/custom-editor.ts +++ b/packages/coding-agent/src/modes/components/custom-editor.ts @@ -439,6 +439,14 @@ export class CustomEditor extends Editor { * assembled payload here; the empty-paste / image-path branches must see the full content, * not the raw single-chunk byte sequence. */ #pasteHandler = new BracketedPasteHandler(); + /** Number of async pastes (clipboard-image reads / image-path attachments) currently in flight. + * While > 0, `handleInput` queues subsequent keystrokes into {@link #pendingInput} instead of + * dispatching them so a trailing `Enter` after `Cmd+V` can't submit before the image lands on + * `pendingImages` (Codex PR #3602 review). */ + #pasteInFlight = 0; + /** Input chunks deferred behind an in-flight paste, drained in FIFO order once the paste + * count returns to zero. */ + #pendingInput: string[] = []; /** Spaces actually inserted in the current run; tracked back out when a hold is recognized. */ #spaceRunInserted = 0; /** Consecutive "mechanical" deltas (fast + steady); a sustained run of these confirms a held bar. */ @@ -592,7 +600,34 @@ export class CustomEditor extends Editor { this.onSpaceHoldEnd?.(); } + /** Decrement {@link #pasteInFlight} once an async paste settles and, when the count returns + * to zero, drain {@link #pendingInput} through `handleInput` so requeueing still works if a + * drained chunk triggers another async paste. Bound member so it can be passed straight to + * `Promise.then(callback, callback)`. */ + #onPasteSettled = (): void => { + this.#pasteInFlight--; + if (this.#pasteInFlight > 0) return; + const drained = this.#pendingInput.splice(0); + for (const chunk of drained) this.handleInput(chunk); + }; + + /** Track `promise` as an in-flight paste so subsequent `handleInput` calls queue behind it, + * then drain the queue once it settles. Codex PR #3602 review: without this, a trailing + * keystroke (Enter most painfully) in the same stdin read processes synchronously while the + * clipboard read is still pending — submit fires with the text but `pendingImages` is still + * empty and the image lands on the *next* draft instead. */ + #trackAsyncPaste(promise: Promise): void { + this.#pasteInFlight++; + void promise.then(this.#onPasteSettled, this.#onPasteSettled); + } + handleInput(data: string): void { + // Serialize behind any in-flight async paste so a trailing Enter / follow-up key can't + // submit before the clipboard image reaches `pendingImages` (Codex PR #3602 review). + if (this.#pasteInFlight > 0) { + this.#pendingInput.push(data); + return; + } const kittyParsed = parseKittySequence(data); if (kittyParsed && (kittyParsed.modifier & 64) !== 0 && this.onCapsLock) { // Caps Lock is modifier bit 64 @@ -617,19 +652,27 @@ export class CustomEditor extends Editor { if (paste.pasteContent === undefined) return; // still buffering — wait for end marker const content = paste.pasteContent; const remaining = paste.remaining; + // Queue any trailing bytes from the same read (typically a follow-up keystroke such as + // Enter that the user pressed right after Cmd+V) so they only fire *after* the paste + // completes — fixes the race where submit runs against an empty `pendingImages`. + if (remaining.length > 0) this.#pendingInput.push(remaining); if (content.length === 0 && this.onPasteImage) { - void this.onPasteImage(); - } else { - const imagePaths = extractImagePastePathsFromText(content); - if (imagePaths && this.onPasteImagePath) { - void (async () => { - for (const p of imagePaths) await this.onPasteImagePath?.(p); - })(); - } else { - this.pasteText(content); - } + this.#trackAsyncPaste(Promise.resolve(this.onPasteImage())); + return; } - if (remaining.length > 0) this.handleInput(remaining); + const imagePaths = extractImagePastePathsFromText(content); + if (imagePaths && this.onPasteImagePath) { + this.#trackAsyncPaste( + (async () => { + for (const p of imagePaths) await this.onPasteImagePath?.(p); + })(), + ); + return; + } + this.pasteText(content); + // No async paste was started; drain the queued trailing bytes ourselves. + const drained = this.#pendingInput.splice(0); + for (const chunk of drained) this.handleInput(chunk); return; } diff --git a/packages/coding-agent/test/issue-3601-repro.test.ts b/packages/coding-agent/test/issue-3601-repro.test.ts index d10a71d68..7581af55d 100644 --- a/packages/coding-agent/test/issue-3601-repro.test.ts +++ b/packages/coding-agent/test/issue-3601-repro.test.ts @@ -176,6 +176,44 @@ describe("CustomEditor empty bracketed paste (issue #3601)", () => { expect(pasteText).toHaveBeenCalledTimes(1); expect(pasteText).toHaveBeenCalledWith("hello world"); }); + + it("defers a trailing keystroke after a Cmd+V empty paste until the image attach settles (Codex PR #3602 review)", async () => { + // Codex review: the empty bracketed paste plus a follow-up key (the + // user hits Enter right after Cmd+V) used to race — `onPasteImage` + // was fire-and-forget, so `\r` dispatched synchronously and submit + // ran against an empty `pendingImages`. The post-fix path queues + // trailing bytes behind the in-flight paste; the trailing key only + // dispatches after the paste promise settles. + const { editor } = createCtx(); + const { promise: imageAttached, resolve: completePaste } = Promise.withResolvers(); + const callOrder: string[] = []; + editor.onPasteImage = () => { + callOrder.push("paste:start"); + return imageAttached; + }; + // Spy a custom key handler for `enter` so we can observe submit ordering + // without standing up the full submit machinery. + const onEnter = vi.fn(() => callOrder.push("enter")); + editor.setCustomKeyHandler("enter", onEnter); + + // Single read carrying both the empty bracketed paste AND the trailing CR. + editor.handleInput(`${BRACKETED_PASTE_START}${BRACKETED_PASTE_END}\r`); + + // Paste started, Enter MUST NOT have fired yet (it would submit pre-image). + expect(callOrder).toEqual(["paste:start"]); + expect(onEnter).not.toHaveBeenCalled(); + + // Settle the clipboard image read; the queued Enter should now drain through. + completePaste(true); + await imageAttached; + // Two microtasks: one for `Promise.resolve(onPasteImage()).then(#onPasteSettled)`, + // one for the synchronous drain that runs inside `#onPasteSettled`. + await Promise.resolve(); + await Promise.resolve(); + + expect(callOrder).toEqual(["paste:start", "enter"]); + expect(onEnter).toHaveBeenCalledTimes(1); + }); }); describe("InputController + empty bracketed paste end-to-end (issue #3601)", () => {