From f02ab20acb5fd4d126ee8ed3cec4bca36b60f6a2 Mon Sep 17 00:00:00 2001 From: cognitive <152830360+METAeuPHORIC@users.noreply.github.com> Date: Wed, 13 May 2026 15:28:31 +0000 Subject: [PATCH] fix(coding-agent/tui): refreshed pending bar on tagged-custom dequeue MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Op: correct Restores: ref:44e5e0bb8 — queued /skill: chip lifecycle parity with plain-text steer EventController.#handleMessageStart now mirrors the user-role refresh in the custom branch, gated on readPendingDisplayTag(details). Without this, AgentSession's tag-keyed dequeue mutated #steeringMessages / #followUpMessages correctly but pendingMessagesContainer kept painting the stale chip until an unrelated trigger (next user submit, dequeue key, compaction flush) fired a refresh. Non-queued custom variants (ttsr-injection, irc:*, async-result, hookMessage) skip the refresh — they never registered a pending chip, so rebuilding pendingMessagesContainer for them would be pure waste. Pairs with the existing E4 (array splice) regression — the new E10 covers the UI-refresh side of the same dequeue event with both positive and negative gate assertions. Co-Authored-By: chatgpt-codex-connector[bot] (P2 review on PR #1043) --- .../src/modes/controllers/event-controller.ts | 13 ++- .../test/input-controller-skill-queue.test.ts | 106 +++++++++++++++++- 2 files changed, 117 insertions(+), 2 deletions(-) diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 75f56aca3..02a7ba4af 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -15,7 +15,7 @@ import { getSymbolTheme, theme } from "../../modes/theme/theme"; import type { InteractiveModeContext, TodoPhase } from "../../modes/types"; import type { AgentSessionEvent } from "../../session/agent-session"; import { calculatePromptTokens } from "../../session/compaction/compaction"; -import { isSilentAbort } from "../../session/messages"; +import { isSilentAbort, readPendingDisplayTag } from "../../session/messages"; import type { ExitPlanModeDetails } from "../../tools"; type AgentSessionEventKind = AgentSessionEvent["type"]; @@ -179,6 +179,17 @@ export class EventController { this.#renderedCustomMessages.add(signature); this.#resetReadGroup(); this.ctx.addMessageToChat(event.message); + // Tag-keyed pending-bar refresh: when AgentSession.#handleAgentEvent + // spliced this dequeued custom message out of #steeringMessages / + // #followUpMessages (it ran before this emit), the array state is + // already correct — pendingMessagesContainer just needs to be + // re-rendered to match. Gated on tag presence so non-queued customs + // (ttsr-injection, irc:*, async-result, hookMessage) skip the + // rebuild; their dispatch path never registered a pending chip. + // Mirrors the user-role refresh at the bottom of this function. + if (event.message.role === "custom" && readPendingDisplayTag(event.message.details)) { + this.ctx.updatePendingMessagesDisplay(); + } this.ctx.ui.requestRender(); } else if (event.message.role === "user") { const textContent = this.ctx.getUserMessageText(event.message); 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 9334ac311..6aebdbaf8 100644 --- a/packages/coding-agent/test/input-controller-skill-queue.test.ts +++ b/packages/coding-agent/test/input-controller-skill-queue.test.ts @@ -23,10 +23,11 @@ import { Agent } from "@oh-my-pi/pi-agent-core"; import { getBundledModel } from "@oh-my-pi/pi-ai/models"; import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { EventController } from "@oh-my-pi/pi-coding-agent/modes/controllers/event-controller"; 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 { UiHelpers } from "@oh-my-pi/pi-coding-agent/modes/utils/ui-helpers"; -import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session"; +import { AgentSession, type AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session"; import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage"; import { SKILL_PROMPT_MESSAGE_TYPE, @@ -429,3 +430,106 @@ describe("UiHelpers / InputController against the queued-display layer (E8-E9)", expect(followUp).toEqual([]); }); }); + +// ============================================================================ +// E10: EventController refreshes the pending-messages bar on tagged custom +// dequeue. +// +// Regression guard for the Codex P2 review finding on PR #1043: the +// custom-role `message_start` branch in AgentSession.#handleAgentEvent spliced +// the matching entry out of #steeringMessages / #followUpMessages correctly, +// but EventController.#handleMessageStart only called updatePendingMessagesDisplay +// from the `role === "user"` branch. The custom branch — which is where queued +// /skill: invocations flow — never rebuilt `pendingMessagesContainer`, so the +// chip kept painting until an unrelated trigger fired a refresh. +// +// The fix: in EventController's custom branch, when the dequeued message +// carries the `__pendingDisplayTag` (proof it was queued via +// enqueueCustomMessageDisplay), call updatePendingMessagesDisplay() before +// requestRender(). E10 covers both gate branches: +// - positive: tagged custom -> refresh fires once +// - negative: untagged custom (ttsr-injection, irc:*, async-result, hookMessage) +// -> refresh NOT fired (over-refresh guard) +// ============================================================================ + +function createEventControllerFixtureForE10() { + const updatePendingMessagesDisplay = vi.fn(); + const addMessageToChat = vi.fn(); + const requestRender = vi.fn(); + const ctx = { + isInitialized: true, + init: vi.fn(async () => {}), + ui: { requestRender }, + statusLine: { invalidate: vi.fn() }, + updateEditorTopBorder: vi.fn(), + addMessageToChat, + updatePendingMessagesDisplay, + session: {}, + } as unknown as InteractiveModeContext; + + const controller = new EventController(ctx); + return { controller, updatePendingMessagesDisplay, addMessageToChat }; +} + +describe("EventController custom-role dequeue refresh (E10)", () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("E10: message_start with role=custom refreshes pending bar ONLY when __pendingDisplayTag is present", async () => { + const { controller, updatePendingMessagesDisplay, addMessageToChat } = + createEventControllerFixtureForE10(); + + // Positive case: tagged custom => refresh fires exactly once. The tag is the + // unambiguous signal "this message was queued via enqueueCustomMessageDisplay"; + // AgentSession.#handleAgentEvent has already spliced the matching entry out of + // the display arrays (ran before this emit), so the rebuild repaints the now- + // correct queue state. + const taggedEvent: Extract = { + type: "message_start", + message: { + role: "custom", + customType: SKILL_PROMPT_MESSAGE_TYPE, + content: "first", + display: true, + details: { + __pendingDisplayTag: "sk-test-0", + name: "foo", + path: "/s.md", + args: "bar", + lineCount: 1, + } satisfies SkillPromptDetails, + timestamp: Date.now(), + }, + }; + await controller.handleEvent(taggedEvent); + expect(updatePendingMessagesDisplay).toHaveBeenCalledTimes(1); + // Chat rendering still ran — refresh is additive, not a replacement for the + // chat path. + expect(addMessageToChat).toHaveBeenCalledTimes(1); + + // Negative case: untagged custom => refresh NOT fired. Over-refresh guard. + // Non-queued customs (ttsr-injection, irc:*, async-result, hookMessage) never + // registered a pending chip, so rebuilding pendingMessagesContainer for them + // would be pure waste. Distinct timestamp avoids the #renderedCustomMessages + // signature-dedup early-return. + const untaggedEvent: Extract = { + type: "message_start", + message: { + role: "custom", + customType: SKILL_PROMPT_MESSAGE_TYPE, + content: "second", + display: true, + details: undefined, + timestamp: Date.now() + 1, + }, + }; + await controller.handleEvent(untaggedEvent); + // Still exactly 1 — no additional call from the untagged path. + expect(updatePendingMessagesDisplay).toHaveBeenCalledTimes(1); + // Chat rendering still ran for the untagged custom (the chat-add path is + // unconditional inside the custom branch; only the pending-bar refresh is + // tag-gated). + expect(addMessageToChat).toHaveBeenCalledTimes(2); + }); +});