fix(coding-agent-turn-interrupt/queue-ux): resolved steering abort state

- Replaced queued-message interrupt flow with session abort calls on empty submit and escape.
- Removed interrupting state and notifyInterrupting teardown paths from abort handling.
- Updated AgentSession queue operations to use shared steering and follow-up queue views.
- Propagated isAborting through session state and collab payloads to suppress late updates.
This commit is contained in:
can1357
2026-06-13 17:19:57 +02:00
parent 3def4a6979
commit 705750453d
23 changed files with 440 additions and 1304 deletions
@@ -1,23 +1,11 @@
/**
* Phase 6 — E layer.
* Skill/custom queued-message display contracts.
*
* Tests the skill-queue + custom-role dequeue contract that ties together:
* - InputController.#invokeSkillCommand (tag generation when streaming);
* - AgentSession.enqueueCustomMessageDisplay + #handleAgentEvent's
* custom-role `message_start` dequeue;
* - UiHelpers.updatePendingMessagesDisplay (compact slash-form rendering);
* - InputController.restoreQueuedMessagesToEditor (recovery of the slash-form
* into the editor).
*
* Tests split into:
* - E1-E3: InputController-side tag generation, stubbed session;
* - E4-E7: Real AgentSession driving synthetic `message_start` events
* for the tag-based custom-role dequeue;
* - E8: real UiHelpers render against a queued-display entry;
* - E9: real InputController.restoreQueuedMessagesToEditor.
* Custom queued chips now ride on the queued AgentMessage itself via
* details.__queueChipText. The session derives pending display directly from
* the agent-core queue; there is no separate display mirror to splice.
*/
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
import * as fs from "node:fs";
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 { getBundledModel } from "@oh-my-pi/pi-catalog/models";
@@ -35,27 +23,26 @@ import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manage
import { Container } from "@oh-my-pi/pi-tui";
import { TempDir } from "@oh-my-pi/pi-utils";
// ============================================================================
// Shared helpers
// ============================================================================
function writeSkillFile(dir: string, skillName: string, body: string): string {
const skillPath = path.join(dir, `${skillName}.md`);
fs.writeFileSync(skillPath, `---\nname: ${skillName}\n---\n${body}\n`);
return skillPath;
}
// ============================================================================
// E1-E3: InputController tag generation with a stubbed session.
// ============================================================================
type StubEditor = {
setText: (text: string) => void;
getText: () => string;
addToHistory: ReturnType<typeof vi.fn>;
addToHistory: Mock<(...args: unknown[]) => unknown>;
onSubmit?: (text: string) => Promise<void>;
};
type PromptCustomMessage = Mock<
(
message: { details: SkillPromptDetails },
options?: { streamingBehavior?: "steer" | "followUp"; queueChipText?: string },
) => Promise<void>
>;
async function writeSkillFile(dir: string, skillName: string, body: string): Promise<string> {
const skillPath = path.join(dir, `${skillName}.md`);
await Bun.write(skillPath, `---\nname: ${skillName}\n---\n${body}\n`);
return skillPath;
}
function createStubInputControllerContext(opts: { skillCommands: Map<string, string>; isStreaming: boolean }) {
let editorText = "";
const editor: StubEditor = {
@@ -67,10 +54,7 @@ function createStubInputControllerContext(opts: { skillCommands: Map<string, str
},
addToHistory: vi.fn(),
};
const enqueueCustomMessageDisplay = vi.fn((_text: string, _mode: "steer" | "followUp") => "sk-test-0");
// Annotate parameters so `mock.calls[N]` is typed as a tuple (not `[]`) and
// `message` carries required skill prompt details for assertion below.
const promptCustomMessage = vi.fn(async (_message: { details: SkillPromptDetails }, _options?: unknown) => {});
const promptCustomMessage: PromptCustomMessage = vi.fn(async () => {});
const prompt = vi.fn(async (_text: string, _options?: unknown) => {});
const handleGoalModeCommand = vi.fn(async (_rest?: string) => {});
const updatePendingMessagesDisplay = vi.fn();
@@ -87,7 +71,6 @@ function createStubInputControllerContext(opts: { skillCommands: Map<string, str
isBashRunning: false,
isEvalRunning: false,
extensionRunner: undefined,
enqueueCustomMessageDisplay,
prompt,
promptCustomMessage,
},
@@ -98,7 +81,6 @@ function createStubInputControllerContext(opts: { skillCommands: Map<string, str
handleGoalModeCommand,
goalModeEnabled: false,
updatePendingMessagesDisplay,
// Defaults that InputController touches on submit but don't matter here.
isBashMode: false,
isPythonMode: false,
pendingImages: [],
@@ -109,16 +91,24 @@ function createStubInputControllerContext(opts: { skillCommands: Map<string, str
withLocalSubmission: async (_text: string, fn: () => unknown) => fn(),
} as unknown as InteractiveModeContext;
return { ctx, editor, enqueueCustomMessageDisplay, prompt, promptCustomMessage, handleGoalModeCommand };
return {
ctx,
editor,
prompt,
promptCustomMessage,
handleGoalModeCommand,
updatePendingMessagesDisplay,
requestRender,
};
}
describe("InputController #invokeSkillCommand (E1-E3)", () => {
describe("InputController skill queue chip metadata", () => {
let tempDir: TempDir;
let skillCommands: Map<string, string>;
beforeEach(() => {
beforeEach(async () => {
tempDir = TempDir.createSync("@pi-skill-queue-stub-");
const skillPath = writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
const skillPath = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
skillCommands = new Map<string, string>([["skill:test-skill", skillPath]]);
});
@@ -127,60 +117,48 @@ describe("InputController #invokeSkillCommand (E1-E3)", () => {
vi.restoreAllMocks();
});
it("E1: streaming + steer -> enqueueCustomMessageDisplay called and details.__pendingDisplayTag set", async () => {
const { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage } = createStubInputControllerContext({
skillCommands,
isStreaming: true,
});
it("passes slash-form queueChipText for streaming skill steers", async () => {
const { ctx, editor, promptCustomMessage, updatePendingMessagesDisplay, requestRender } =
createStubInputControllerContext({ skillCommands, isStreaming: true });
const controller = new InputController(ctx);
controller.setupEditorSubmitHandler();
editor.setText("/skill:test-skill arg1 arg2");
await editor.onSubmit?.("/skill:test-skill arg1 arg2");
expect(enqueueCustomMessageDisplay).toHaveBeenCalledTimes(1);
expect(enqueueCustomMessageDisplay).toHaveBeenCalledWith("/skill:test-skill arg1 arg2", "steer");
expect(promptCustomMessage).toHaveBeenCalledTimes(1);
const firstCall = promptCustomMessage.mock.calls[0];
expect(firstCall).toBeDefined();
if (!firstCall) {
throw new Error("expected promptCustomMessage to be called");
}
const messageArg = firstCall[0];
expect(messageArg.details.__pendingDisplayTag).toBe("sk-test-0");
expect(promptCustomMessage.mock.calls[0]?.[1]).toEqual({
streamingBehavior: "steer",
queueChipText: "/skill:test-skill arg1 arg2",
});
expect(promptCustomMessage.mock.calls[0]?.[0].details.__queueChipText).toBeUndefined();
expect(updatePendingMessagesDisplay).toHaveBeenCalledTimes(1);
expect(requestRender).toHaveBeenCalledTimes(1);
});
it("E2: streaming + followUp -> enqueueCustomMessageDisplay called with mode 'followUp', tag embedded", async () => {
const { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage } = createStubInputControllerContext({
it("passes slash-form queueChipText for streaming skill follow-ups", async () => {
const { ctx, editor, promptCustomMessage } = createStubInputControllerContext({
skillCommands,
isStreaming: true,
});
const controller = new InputController(ctx);
editor.setText("/skill:test-skill arg1 arg2");
// `handleFollowUp` is the Ctrl+Enter dispatcher; it routes through the same
// `#invokeSkillCommand` helper with mode "followUp".
await controller.handleFollowUp();
expect(enqueueCustomMessageDisplay).toHaveBeenCalledWith("/skill:test-skill arg1 arg2", "followUp");
const firstCall = promptCustomMessage.mock.calls[0];
expect(firstCall).toBeDefined();
if (!firstCall) {
throw new Error("expected promptCustomMessage to be called");
}
const messageArg = firstCall[0];
expect(messageArg.details.__pendingDisplayTag).toBe("sk-test-0");
expect(promptCustomMessage.mock.calls[0]?.[1]).toEqual({
streamingBehavior: "followUp",
queueChipText: "/skill:test-skill arg1 arg2",
});
});
it("E2b: streaming follow-up applies builtin slash commands instead of queueing them", async () => {
it("streaming follow-up applies builtin slash commands instead of queueing them", async () => {
const { ctx, editor, prompt, handleGoalModeCommand } = createStubInputControllerContext({
skillCommands,
isStreaming: true,
});
const controller = new InputController(ctx);
editor.setText("/goal set Ship the release");
await controller.handleFollowUp();
@@ -189,32 +167,25 @@ describe("InputController #invokeSkillCommand (E1-E3)", () => {
expect(editor.getText()).toBe("");
});
it("E3: not streaming -> enqueueCustomMessageDisplay NOT called and tag absent", async () => {
const { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage } = createStubInputControllerContext({
it("idle skill prompt still leaves queueChipText out of persisted details", async () => {
const { ctx, editor, promptCustomMessage } = createStubInputControllerContext({
skillCommands,
isStreaming: false,
});
const controller = new InputController(ctx);
controller.setupEditorSubmitHandler();
editor.setText("/skill:test-skill arg1 arg2");
await editor.onSubmit?.("/skill:test-skill arg1 arg2");
expect(enqueueCustomMessageDisplay).not.toHaveBeenCalled();
const firstCall = promptCustomMessage.mock.calls[0];
expect(firstCall).toBeDefined();
if (!firstCall) {
throw new Error("expected promptCustomMessage to be called");
}
const messageArg = firstCall[0];
expect(messageArg.details.__pendingDisplayTag).toBeUndefined();
expect(promptCustomMessage.mock.calls[0]?.[1]).toEqual({
streamingBehavior: "steer",
queueChipText: "/skill:test-skill arg1 arg2",
});
expect(promptCustomMessage.mock.calls[0]?.[0].details.__queueChipText).toBeUndefined();
});
});
// ============================================================================
// E4-E7: Real AgentSession driving synthetic `message_start` events.
// ============================================================================
interface SessionFixture {
tempDir: TempDir;
authStorage: AuthStorage;
@@ -248,24 +219,24 @@ async function createRealSession(): Promise<SessionFixture> {
return { tempDir, authStorage, session };
}
/** Emit a `message_start` for a custom message whose `details` carries the supplied tag. */
function emitCustomMessageStart(session: AgentSession, content: string, tag?: string): void {
const details: { __pendingDisplayTag?: string } | undefined =
tag === undefined ? undefined : { __pendingDisplayTag: tag };
session.agent.emitExternalEvent({
type: "message_start",
message: {
role: "custom",
customType: SKILL_PROMPT_MESSAGE_TYPE,
content,
display: true,
details,
timestamp: Date.now(),
},
function queueCustomSteer(session: AgentSession, chip: string, content = "skill body"): void {
session.agent.steer({
role: "custom",
customType: SKILL_PROMPT_MESSAGE_TYPE,
content,
display: true,
details: {
name: "foo",
path: "/s.md",
args: "bar",
lineCount: 1,
__queueChipText: chip,
} satisfies SkillPromptDetails,
timestamp: Date.now(),
});
}
describe("AgentSession custom-role tag dequeue (E4-E7)", () => {
describe("AgentSession derived queued custom display", () => {
let fixture: SessionFixture | undefined;
afterEach(async () => {
@@ -278,95 +249,42 @@ describe("AgentSession custom-role tag dequeue (E4-E7)", () => {
vi.restoreAllMocks();
});
it("E4: message_start with role=custom + matching tag removes the tagged display entry", async () => {
it("derives queued custom chip text directly from the agent steering queue", async () => {
fixture = await createRealSession();
const { session } = fixture;
const tag = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
expect(tag).not.toBe("");
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar"]);
emitCustomMessageStart(session, "irrelevant content", tag);
await Promise.resolve();
await Promise.resolve();
queueCustomSteer(session, "/skill:foo bar");
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar"]);
expect(session.queuedMessageCount).toBe(1);
});
it("excludes display-suppressed custom messages from pending chips and counts", async () => {
fixture = await createRealSession();
const { session } = fixture;
session.agent.steer({
role: "custom",
customType: "internal",
content: "hidden",
display: false,
details: { __queueChipText: "hidden" },
timestamp: Date.now(),
});
expect(session.getQueuedMessages().steering).toEqual([]);
// And internal queue counters reflect the empty steer/followUp arrays. The
// pending-next-turn store stays at zero too because this test never queued one.
expect(session.queuedMessageCount).toBe(0);
});
it("E5: message_start with role=custom but no tag is a no-op", async () => {
it("popLastQueuedMessage restores chip text and removes the core queue entry", async () => {
fixture = await createRealSession();
const { session } = fixture;
session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
const beforeCount = session.queuedMessageCount;
expect(beforeCount).toBe(1);
queueCustomSteer(session, "/skill:foo bar");
emitCustomMessageStart(session, "irrelevant content"); // no tag
await Promise.resolve();
await Promise.resolve();
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar"]);
expect(session.queuedMessageCount).toBe(beforeCount);
});
it("E6: two queued skills with identical args text are dequeued independently by tag", async () => {
fixture = await createRealSession();
const { session } = fixture;
const tag1 = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
const tag2 = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
expect(tag1).not.toBe(tag2);
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar", "/skill:foo bar"]);
// Consume the SECOND-enqueued tag. After dequeue, the SURVIVING entry must be
// the one that was added FIRST — proves the dequeue keys off `tag`, not off
// `indexOf(text)` (which would always have removed the first match).
emitCustomMessageStart(session, "any", tag2);
await Promise.resolve();
await Promise.resolve();
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar"]);
// Now dequeue the first; nothing left.
emitCustomMessageStart(session, "any", tag1);
await Promise.resolve();
await Promise.resolve();
expect(session.getQueuedMessages().steering).toEqual([]);
});
it("E7: popLastQueuedMessage on a tagged entry leaves no orphan tag state", async () => {
fixture = await createRealSession();
const { session } = fixture;
const firstTag = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
const popped = session.popLastQueuedMessage();
expect(popped?.text).toBe("/skill:foo bar");
expect(session.getQueuedMessages().steering).toEqual([]);
// Push a NEW tagged entry with the same text. Emitting `message_start` for the
// FIRST (popped) tag must be a no-op — the dequeue cannot reach into the new
// entry because the popped tag died with its record.
const secondTag = session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
expect(secondTag).not.toBe(firstTag);
emitCustomMessageStart(session, "any", firstTag);
await Promise.resolve();
await Promise.resolve();
expect(session.getQueuedMessages().steering).toEqual(["/skill:foo bar"]);
// Sanity: the second tag still works.
emitCustomMessageStart(session, "any", secondTag);
await Promise.resolve();
await Promise.resolve();
expect(session.popLastQueuedMessage()?.text).toBe("/skill:foo bar");
expect(session.getQueuedMessages().steering).toEqual([]);
});
});
// ============================================================================
// E8-E9: Real UiHelpers / InputController against a session populated through
// enqueueCustomMessageDisplay.
// ============================================================================
function createStubInteractiveModeContextForUiHelpers(session: AgentSession) {
let editorText = "";
const editor: StubEditor = {
@@ -399,19 +317,10 @@ function createStubInteractiveModeContextForUiHelpers(session: AgentSession) {
return { ctx, editor, pendingMessagesContainer };
}
describe("UiHelpers / InputController against the queued-display layer (E8-E9)", () => {
describe("UiHelpers / InputController against derived queued custom display", () => {
let fixture: SessionFixture | undefined;
beforeEach(async () => {
// E8 invokes the real `theme.fg(...)` codepath inside
// updatePendingMessagesDisplay; without an initialized theme module the
// global `theme` variable is undefined. Installs `dark` per-test —
// matches the established suite convention used by other test files
// (bash-execution-clamp.test.ts, bash-execution-sixel.test.ts) where
// `dark` is the agreed default for every test that needs a theme.
// No `afterEach` restore is required by that convention; the theme
// module exposes no reset API, and `dark` is the suite-wide assumed
// post-state.
const themeInstance = await getThemeByName("dark");
expect(themeInstance).toBeDefined();
setThemeInstance(themeInstance!);
@@ -427,60 +336,35 @@ describe("UiHelpers / InputController against the queued-display layer (E8-E9)",
vi.restoreAllMocks();
});
it("E8: updatePendingMessagesDisplay renders the compact slash form for queued skills", async () => {
it("renders the compact slash form for queued skills", async () => {
fixture = await createRealSession();
const { session } = fixture;
session.enqueueCustomMessageDisplay("/skill:test-skill arg1 arg2", "steer");
queueCustomSteer(session, "/skill:test-skill arg1 arg2");
const { ctx, pendingMessagesContainer } = createStubInteractiveModeContextForUiHelpers(session);
const uiHelpers = new UiHelpers(ctx);
uiHelpers.updatePendingMessagesDisplay();
// Render the container at a generous width and assert the compact slash-form
// chip appears verbatim. Matches the user-facing "Steer: /skill:..." format.
const rendered = pendingMessagesContainer.render(120).join("\n");
expect(rendered).toMatch(/Steer: \/skill:test-skill arg1 arg2/);
});
it("E9: restoreQueuedMessagesToEditor recovers the compact slash form into the editor and clears the queue", async () => {
it("restores the compact slash form into the editor and clears the queue", async () => {
fixture = await createRealSession();
const { session } = fixture;
session.enqueueCustomMessageDisplay("/skill:test-skill arg1 arg2", "steer");
queueCustomSteer(session, "/skill:test-skill arg1 arg2");
const { ctx, editor } = createStubInteractiveModeContextForUiHelpers(session);
const controller = new InputController(ctx);
const count = controller.restoreQueuedMessagesToEditor();
expect(count).toBe(1);
expect(editor.getText()).toBe("/skill:test-skill arg1 arg2");
// Queue cleared on both arrays.
const { steering, followUp } = session.getQueuedMessages();
expect(steering).toEqual([]);
expect(followUp).toEqual([]);
expect(session.getQueuedMessages()).toEqual({ steering: [], followUp: [] });
});
});
// ============================================================================
// 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() {
function createEventControllerFixture() {
const updatePendingMessagesDisplay = vi.fn();
const addMessageToChat = vi.fn();
const requestRender = vi.fn();
@@ -503,20 +387,14 @@ function createEventControllerFixtureForE10() {
return { controller, updatePendingMessagesDisplay, addMessageToChat };
}
describe("EventController custom-role dequeue refresh (E10)", () => {
describe("EventController custom queued-message refresh", () => {
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<AgentSessionEvent, { type: "message_start" }> = {
it("refreshes the pending bar only for custom messages carrying __queueChipText", async () => {
const { controller, updatePendingMessagesDisplay, addMessageToChat } = createEventControllerFixture();
const queuedEvent: Extract<AgentSessionEvent, { type: "message_start" }> = {
type: "message_start",
message: {
role: "custom",
@@ -524,7 +402,7 @@ describe("EventController custom-role dequeue refresh (E10)", () => {
content: "first",
display: true,
details: {
__pendingDisplayTag: "sk-test-0",
__queueChipText: "/skill:foo bar",
name: "foo",
path: "/s.md",
args: "bar",
@@ -533,18 +411,11 @@ describe("EventController custom-role dequeue refresh (E10)", () => {
timestamp: Date.now(),
},
};
await controller.handleEvent(taggedEvent);
await controller.handleEvent(queuedEvent);
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<AgentSessionEvent, { type: "message_start" }> = {
const unqueuedEvent: Extract<AgentSessionEvent, { type: "message_start" }> = {
type: "message_start",
message: {
role: "custom",
@@ -555,12 +426,9 @@ describe("EventController custom-role dequeue refresh (E10)", () => {
timestamp: Date.now() + 1,
},
};
await controller.handleEvent(untaggedEvent);
// Still exactly 1 — no additional call from the untagged path.
await controller.handleEvent(unqueuedEvent);
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);
});
});