fix(coding-agent/tui): rendered queued /skill: as compact pending chip
This commit is contained in:
@@ -443,6 +443,15 @@ export class InputController {
|
||||
args: args || undefined,
|
||||
lineCount: body ? body.split("\n").length : 0,
|
||||
};
|
||||
// When the agent is streaming, register the compact slash-form text as
|
||||
// the pending-display twin BEFORE dispatching the CustomMessage. The
|
||||
// returned tag is embedded in details so AgentSession.#handleAgentEvent
|
||||
// can remove the matching display entry when the agent consumes this
|
||||
// message (mirrors the user-message dequeue path).
|
||||
if (this.ctx.session.isStreaming) {
|
||||
const tag = this.ctx.session.enqueueCustomMessageDisplay(text, streamingBehavior);
|
||||
details.__pendingDisplayTag = tag;
|
||||
}
|
||||
await this.ctx.session.promptCustomMessage(
|
||||
{
|
||||
customType: SKILL_PROMPT_MESSAGE_TYPE,
|
||||
@@ -453,6 +462,10 @@ export class InputController {
|
||||
},
|
||||
{ streamingBehavior },
|
||||
);
|
||||
if (this.ctx.session.isStreaming) {
|
||||
this.ctx.updatePendingMessagesDisplay();
|
||||
this.ctx.ui.requestRender();
|
||||
}
|
||||
} catch (err) {
|
||||
this.ctx.showError(`Failed to load skill: ${err instanceof Error ? err.message : String(err)}`);
|
||||
}
|
||||
|
||||
@@ -174,6 +174,7 @@ import {
|
||||
convertToLlm,
|
||||
type FileMentionMessage,
|
||||
type PythonExecutionMessage,
|
||||
readPendingDisplayTag,
|
||||
} from "./messages";
|
||||
import { formatSessionDumpText } from "./session-dump-format";
|
||||
import type {
|
||||
@@ -594,6 +595,13 @@ function extractPermissionLocations(args: unknown, cwd: string): { path: string;
|
||||
// AgentSession Class
|
||||
// ============================================================================
|
||||
|
||||
/** Internal record stored in the steering/followUp display queues. The optional
|
||||
* `tag` is set only by `enqueueCustomMessageDisplay` (used for skill-prompt
|
||||
* custom messages queued during streaming) and is matched by the custom-role
|
||||
* `message_start` dequeue branch; user-message pushes leave it undefined and
|
||||
* rely on the existing text-equality match. */
|
||||
type QueuedDisplayEntry = { text: string; tag?: string };
|
||||
|
||||
export class AgentSession {
|
||||
readonly agent: Agent;
|
||||
readonly sessionManager: SessionManager;
|
||||
@@ -612,10 +620,15 @@ export class AgentSession {
|
||||
#unsubscribeAgent?: () => void;
|
||||
#eventListeners: AgentSessionEventListener[] = [];
|
||||
|
||||
/** Tracks pending steering messages for UI display. Removed when delivered. */
|
||||
#steeringMessages: string[] = [];
|
||||
/** Tracks pending follow-up messages for UI display. Removed when delivered. */
|
||||
#followUpMessages: string[] = [];
|
||||
/** Tracks pending steering messages for UI display. Removed when delivered.
|
||||
* Entry shape: `{ text }` for plain-text steers (user-message dequeue
|
||||
* matches by `.text`); `{ text, tag }` for queued custom messages (skill
|
||||
* invocations dispatched while streaming) — the custom-role dequeue
|
||||
* matches by `.tag` so duplicate-args queued skills cannot collide. */
|
||||
#steeringMessages: QueuedDisplayEntry[] = [];
|
||||
/** Tracks pending follow-up messages for UI display. Removed when delivered.
|
||||
* See `#steeringMessages` for entry shape. */
|
||||
#followUpMessages: QueuedDisplayEntry[] = [];
|
||||
/** Messages queued to be included with the next user prompt as context ("asides"). */
|
||||
#pendingNextTurnMessages: CustomMessage[] = [];
|
||||
#scheduledHiddenNextTurnGeneration: number | undefined = undefined;
|
||||
@@ -729,6 +742,11 @@ export class AgentSession {
|
||||
#ttsrRetryToken = 0;
|
||||
#ttsrResumePromise: Promise<void> | undefined = undefined;
|
||||
#ttsrResumeResolve: (() => void) | undefined = undefined;
|
||||
|
||||
/** Monotonic counter for `enqueueCustomMessageDisplay` tag generation;
|
||||
* combined with `Date.now()` so tags stay unique even across rapid
|
||||
* same-tick enqueues. */
|
||||
#customDisplayTagCounter = 0;
|
||||
#postPromptTasks = new Set<Promise<void>>();
|
||||
#postPromptTasksPromise: Promise<void> | undefined = undefined;
|
||||
#postPromptTasksResolve: (() => void) | undefined = undefined;
|
||||
@@ -950,6 +968,28 @@ export class AgentSession {
|
||||
return this.#ttsrAbortPending;
|
||||
}
|
||||
|
||||
/** Register a compact display string for a custom message that the caller is
|
||||
* about to dispatch via `promptCustomMessage` / `sendCustomMessage`.
|
||||
* Returns a stable tag the caller MUST embed in
|
||||
* `CustomMessage.details.__pendingDisplayTag` so the agent-side
|
||||
* `message_start` handler can remove the matching display entry when the
|
||||
* queued message is consumed.
|
||||
*
|
||||
* Does NOT push to the agent's steering/followUp queue — that happens
|
||||
* separately inside `sendCustomMessage`. */
|
||||
enqueueCustomMessageDisplay(text: string, mode: "steer" | "followUp"): string {
|
||||
const tag = `omp-cmd-${Date.now()}-${++this.#customDisplayTagCounter}`;
|
||||
const displayText = text.trim();
|
||||
if (!displayText) return tag;
|
||||
const entry: QueuedDisplayEntry = { text: displayText, tag };
|
||||
if (mode === "steer") {
|
||||
this.#steeringMessages.push(entry);
|
||||
} else {
|
||||
this.#followUpMessages.push(entry);
|
||||
}
|
||||
return tag;
|
||||
}
|
||||
|
||||
getAsyncJobSnapshot(options?: { recentLimit?: number }): AsyncJobSnapshot | null {
|
||||
const manager = AsyncJobManager.instance();
|
||||
if (!manager) return null;
|
||||
@@ -1038,13 +1078,13 @@ export class AgentSession {
|
||||
if (event.type === "message_start" && event.message.role === "user") {
|
||||
const messageText = this.#getUserMessageText(event.message);
|
||||
if (messageText) {
|
||||
// Check steering queue first
|
||||
const steeringIndex = this.#steeringMessages.indexOf(messageText);
|
||||
// Check steering queue first (match by .text on tagged records)
|
||||
const steeringIndex = this.#steeringMessages.findIndex(e => e.text === messageText);
|
||||
if (steeringIndex !== -1) {
|
||||
this.#steeringMessages.splice(steeringIndex, 1);
|
||||
} else {
|
||||
// Check follow-up queue
|
||||
const followUpIndex = this.#followUpMessages.indexOf(messageText);
|
||||
const followUpIndex = this.#followUpMessages.findIndex(e => e.text === messageText);
|
||||
if (followUpIndex !== -1) {
|
||||
this.#followUpMessages.splice(followUpIndex, 1);
|
||||
}
|
||||
@@ -1052,6 +1092,26 @@ export class AgentSession {
|
||||
}
|
||||
}
|
||||
|
||||
// Tag-based dequeue for custom messages (skills queued via promptCustomMessage).
|
||||
// The InputController attached a stable tag via CustomMessage.details when it
|
||||
// registered the display chip; pull it back here to remove the matching entry
|
||||
// from the pending bar atomically with the agent's queue consumption. Match by
|
||||
// tag (not text) — two queued skills with identical args cannot collide.
|
||||
if (event.type === "message_start" && event.message.role === "custom") {
|
||||
const tag = readPendingDisplayTag(event.message.details);
|
||||
if (tag) {
|
||||
const steerIdx = this.#steeringMessages.findIndex(e => e.tag === tag);
|
||||
if (steerIdx !== -1) {
|
||||
this.#steeringMessages.splice(steerIdx, 1);
|
||||
} else {
|
||||
const followUpIdx = this.#followUpMessages.findIndex(e => e.tag === tag);
|
||||
if (followUpIdx !== -1) {
|
||||
this.#followUpMessages.splice(followUpIdx, 1);
|
||||
}
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
// Deobfuscate assistant message content for display emission — the LLM echoes back
|
||||
// obfuscated placeholders, but listeners (TUI, extensions, exporters) must see real
|
||||
// values. The original event.message stays obfuscated so the persistence path below
|
||||
@@ -3719,7 +3779,7 @@ export class AgentSession {
|
||||
*/
|
||||
async #queueSteer(text: string, images?: ImageContent[]): Promise<void> {
|
||||
const displayText = text || (images && images.length > 0 ? "[Image]" : "");
|
||||
this.#steeringMessages.push(displayText);
|
||||
this.#steeringMessages.push({ text: displayText });
|
||||
const content: (TextContent | ImageContent)[] = [{ type: "text", text }];
|
||||
if (images && images.length > 0) {
|
||||
content.push(...images);
|
||||
@@ -3737,7 +3797,7 @@ export class AgentSession {
|
||||
*/
|
||||
async #queueFollowUp(text: string, images?: ImageContent[]): Promise<void> {
|
||||
const displayText = text || (images && images.length > 0 ? "[Image]" : "");
|
||||
this.#followUpMessages.push(displayText);
|
||||
this.#followUpMessages.push({ text: displayText });
|
||||
const content: (TextContent | ImageContent)[] = [{ type: "text", text }];
|
||||
if (images && images.length > 0) {
|
||||
content.push(...images);
|
||||
@@ -3973,8 +4033,8 @@ export class AgentSession {
|
||||
* Useful for restoring to editor when user aborts.
|
||||
*/
|
||||
clearQueue(): { steering: string[]; followUp: string[] } {
|
||||
const steering = [...this.#steeringMessages];
|
||||
const followUp = [...this.#followUpMessages];
|
||||
const steering = this.#steeringMessages.map(e => e.text);
|
||||
const followUp = this.#followUpMessages.map(e => e.text);
|
||||
this.#steeringMessages = [];
|
||||
this.#followUpMessages = [];
|
||||
this.agent.clearAllQueues();
|
||||
@@ -3986,27 +4046,35 @@ export class AgentSession {
|
||||
return this.#steeringMessages.length + this.#followUpMessages.length + this.#pendingNextTurnMessages.length;
|
||||
}
|
||||
|
||||
/** Get pending messages (read-only) */
|
||||
/** Get pending messages (read-only). Returns the public text-only view;
|
||||
* internal `{text, tag?}` records are mapped to `.text` so callers
|
||||
* (`updatePendingMessagesDisplay`, `restoreQueuedMessagesToEditor`) see
|
||||
* the unchanged historical shape. */
|
||||
getQueuedMessages(): { steering: readonly string[]; followUp: readonly string[] } {
|
||||
return { steering: this.#steeringMessages, followUp: this.#followUpMessages };
|
||||
return {
|
||||
steering: this.#steeringMessages.map(e => e.text),
|
||||
followUp: this.#followUpMessages.map(e => e.text),
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Pop the last queued message (steering first, then follow-up).
|
||||
* Used by dequeue keybinding to restore messages to editor one at a time.
|
||||
* Returns the popped entry's `.text`; the tag (if any) dies with the
|
||||
* record — no orphan state can outlive the queue entry.
|
||||
*/
|
||||
popLastQueuedMessage(): string | undefined {
|
||||
// Pop from steering first (LIFO)
|
||||
if (this.#steeringMessages.length > 0) {
|
||||
const message = this.#steeringMessages.pop();
|
||||
const entry = this.#steeringMessages.pop();
|
||||
this.agent.popLastSteer();
|
||||
return message;
|
||||
return entry?.text;
|
||||
}
|
||||
// Then from follow-up
|
||||
if (this.#followUpMessages.length > 0) {
|
||||
const message = this.#followUpMessages.pop();
|
||||
const entry = this.#followUpMessages.pop();
|
||||
this.agent.popLastFollowUp();
|
||||
return message;
|
||||
return entry?.text;
|
||||
}
|
||||
return undefined;
|
||||
}
|
||||
|
||||
@@ -30,6 +30,51 @@ export interface SkillPromptDetails {
|
||||
path: string;
|
||||
args?: string;
|
||||
lineCount: number;
|
||||
/** Internal: tag used by AgentSession to remove the pending-display chip
|
||||
* from `#steeringMessages` / `#followUpMessages` when the agent consumes
|
||||
* this message. Not surfaced to renderers; the `__` prefix signals
|
||||
* "private". Optional — non-streaming skill prompts never set it. Stripped
|
||||
* from persisted `details` by `SessionManager.appendCustomMessageEntry`
|
||||
* via the `INTERNAL_DETAILS_FIELDS` allowlist below. */
|
||||
__pendingDisplayTag?: string;
|
||||
}
|
||||
|
||||
/** Extract the optional `__pendingDisplayTag` field from a CustomMessage's
|
||||
* `details` blob. Safe over `unknown`; returns undefined when the field is
|
||||
* absent or non-string. */
|
||||
export function readPendingDisplayTag(details: unknown): string | undefined {
|
||||
if (typeof details !== "object" || details === null) return undefined;
|
||||
const candidate = (details as { __pendingDisplayTag?: unknown }).__pendingDisplayTag;
|
||||
return typeof candidate === "string" ? candidate : undefined;
|
||||
}
|
||||
|
||||
/** Explicit allowlist of `details` field names that are AgentSession-internal
|
||||
* transient bookkeeping and MUST be removed before SessionManager persists
|
||||
* the CustomMessageEntry to disk. Scoped intentionally narrow: only fields
|
||||
* declared here are stripped. Adding a new entry is a deliberate, reviewed
|
||||
* change — unrelated future payload fields are never silently dropped. */
|
||||
export const INTERNAL_DETAILS_FIELDS = ["__pendingDisplayTag"] as const;
|
||||
|
||||
/** Return a `details` copy with every key in `INTERNAL_DETAILS_FIELDS`
|
||||
* removed. Returns the input unchanged when there is nothing to strip
|
||||
* (null/non-object, or no listed fields present) so callers don't pay a
|
||||
* clone cost on the common path. */
|
||||
export function stripInternalDetailsFields<T>(details: T | undefined): T | undefined {
|
||||
if (details == null || typeof details !== "object") return details;
|
||||
const obj = details as Record<string, unknown>;
|
||||
let hit = false;
|
||||
for (const key of INTERNAL_DETAILS_FIELDS) {
|
||||
if (key in obj) {
|
||||
hit = true;
|
||||
break;
|
||||
}
|
||||
}
|
||||
if (!hit) return details;
|
||||
const cleaned: Record<string, unknown> = { ...obj };
|
||||
for (const key of INTERNAL_DETAILS_FIELDS) {
|
||||
delete cleaned[key];
|
||||
}
|
||||
return cleaned as T;
|
||||
}
|
||||
|
||||
function getPrunedToolResultContent(message: ToolResultMessage): (TextContent | ImageContent)[] {
|
||||
|
||||
@@ -47,6 +47,7 @@ import {
|
||||
type HookMessage,
|
||||
type PythonExecutionMessage,
|
||||
sanitizeRehydratedOpenAIResponsesAssistantMessage,
|
||||
stripInternalDetailsFields,
|
||||
} from "./messages";
|
||||
import type { SessionStorage, SessionStorageWriter } from "./session-storage";
|
||||
import { FileSessionStorage, MemorySessionStorage } from "./session-storage";
|
||||
@@ -2544,7 +2545,10 @@ export class SessionManager {
|
||||
customType,
|
||||
content,
|
||||
display,
|
||||
details,
|
||||
// Drop AgentSession-internal transient fields (allowlist in
|
||||
// `INTERNAL_DETAILS_FIELDS`) before disk persistence. Single
|
||||
// chokepoint covers every CustomMessage write path.
|
||||
details: stripInternalDetailsFields(details),
|
||||
attribution,
|
||||
id: generateId(this.#byId),
|
||||
parentId: this.#leafId,
|
||||
|
||||
@@ -0,0 +1,431 @@
|
||||
/**
|
||||
* Phase 6 — E layer.
|
||||
*
|
||||
* 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.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as path from "node:path";
|
||||
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 { 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 { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import {
|
||||
SKILL_PROMPT_MESSAGE_TYPE,
|
||||
type SkillPromptDetails,
|
||||
} from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
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>;
|
||||
onSubmit?: (text: string) => Promise<void>;
|
||||
};
|
||||
|
||||
function createStubInputControllerContext(opts: {
|
||||
skillCommands: Map<string, string>;
|
||||
isStreaming: boolean;
|
||||
}) {
|
||||
let editorText = "";
|
||||
const editor: StubEditor = {
|
||||
setText(text) {
|
||||
editorText = text;
|
||||
},
|
||||
getText() {
|
||||
return editorText;
|
||||
},
|
||||
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 `[]`) —
|
||||
// avoids TS2352/TS2493 when casting `.calls[0]` to a destructured shape.
|
||||
const promptCustomMessage = vi.fn(
|
||||
async (_message: { details?: SkillPromptDetails }, _options?: unknown) => {},
|
||||
);
|
||||
const updatePendingMessagesDisplay = vi.fn();
|
||||
const requestRender = vi.fn();
|
||||
const showError = vi.fn();
|
||||
|
||||
const ctx = {
|
||||
editor,
|
||||
ui: { requestRender },
|
||||
skillCommands: opts.skillCommands,
|
||||
session: {
|
||||
isStreaming: opts.isStreaming,
|
||||
isCompacting: false,
|
||||
isBashRunning: false,
|
||||
isEvalRunning: false,
|
||||
extensionRunner: undefined,
|
||||
enqueueCustomMessageDisplay,
|
||||
promptCustomMessage,
|
||||
},
|
||||
showError,
|
||||
updatePendingMessagesDisplay,
|
||||
// Defaults that InputController touches on submit but don't matter here.
|
||||
isBashMode: false,
|
||||
isPythonMode: false,
|
||||
pendingImages: [],
|
||||
isBackgrounded: false,
|
||||
loopModeEnabled: false,
|
||||
compactionQueuedMessages: [],
|
||||
locallySubmittedUserSignatures: new Set<string>(),
|
||||
withLocalSubmission: async (_text: string, fn: () => unknown) => fn(),
|
||||
} as unknown as InteractiveModeContext;
|
||||
|
||||
return { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage };
|
||||
}
|
||||
|
||||
describe("InputController #invokeSkillCommand (E1-E3)", () => {
|
||||
let tempDir: TempDir;
|
||||
let skillCommands: Map<string, string>;
|
||||
|
||||
beforeEach(() => {
|
||||
tempDir = TempDir.createSync("@pi-skill-queue-stub-");
|
||||
const skillPath = writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
|
||||
skillCommands = new Map<string, string>([["skill:test-skill", skillPath]]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
tempDir.removeSync();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("E1: streaming + steer -> enqueueCustomMessageDisplay called and details.__pendingDisplayTag set", async () => {
|
||||
const { ctx, editor, enqueueCustomMessageDisplay, promptCustomMessage } =
|
||||
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();
|
||||
// biome-ignore lint/style/noNonNullAssertion: just asserted non-undefined
|
||||
const messageArg = firstCall![0] as { details: SkillPromptDetails };
|
||||
expect(messageArg.details.__pendingDisplayTag).toBe("sk-test-0");
|
||||
});
|
||||
|
||||
it("E2: streaming + followUp -> enqueueCustomMessageDisplay called with mode 'followUp', tag embedded", async () => {
|
||||
const { ctx, editor, enqueueCustomMessageDisplay, 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();
|
||||
// biome-ignore lint/style/noNonNullAssertion: just asserted non-undefined
|
||||
const messageArg = firstCall![0] as { details: SkillPromptDetails };
|
||||
expect(messageArg.details.__pendingDisplayTag).toBe("sk-test-0");
|
||||
});
|
||||
|
||||
it("E3: not streaming -> enqueueCustomMessageDisplay NOT called and tag absent", async () => {
|
||||
const { ctx, editor, enqueueCustomMessageDisplay, 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();
|
||||
// biome-ignore lint/style/noNonNullAssertion: just asserted non-undefined
|
||||
const messageArg = firstCall![0] as { details: SkillPromptDetails };
|
||||
expect(messageArg.details.__pendingDisplayTag).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// ============================================================================
|
||||
// E4-E7: Real AgentSession driving synthetic `message_start` events.
|
||||
// ============================================================================
|
||||
|
||||
interface SessionFixture {
|
||||
tempDir: TempDir;
|
||||
authStorage: AuthStorage;
|
||||
session: AgentSession;
|
||||
}
|
||||
|
||||
async function createRealSession(): Promise<SessionFixture> {
|
||||
const tempDir = TempDir.createSync("@pi-skill-queue-real-");
|
||||
const authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||||
if (!model) throw new Error("Expected built-in anthropic model to exist");
|
||||
|
||||
const agent = new Agent({
|
||||
initialState: {
|
||||
model,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
});
|
||||
|
||||
const session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings: Settings.isolated(),
|
||||
modelRegistry,
|
||||
});
|
||||
|
||||
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(),
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
describe("AgentSession custom-role tag dequeue (E4-E7)", () => {
|
||||
let fixture: SessionFixture | undefined;
|
||||
|
||||
afterEach(async () => {
|
||||
if (fixture) {
|
||||
await fixture.session.dispose();
|
||||
fixture.authStorage.close();
|
||||
fixture.tempDir.removeSync();
|
||||
fixture = undefined;
|
||||
}
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("E4: message_start with role=custom + matching tag removes the tagged display entry", 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();
|
||||
|
||||
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 () => {
|
||||
fixture = await createRealSession();
|
||||
const { session } = fixture;
|
||||
session.enqueueCustomMessageDisplay("/skill:foo bar", "steer");
|
||||
const beforeCount = session.queuedMessageCount;
|
||||
expect(beforeCount).toBe(1);
|
||||
|
||||
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).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.getQueuedMessages().steering).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
// ============================================================================
|
||||
// E8-E9: Real UiHelpers / InputController against a session populated through
|
||||
// enqueueCustomMessageDisplay.
|
||||
// ============================================================================
|
||||
|
||||
function createStubInteractiveModeContextForUiHelpers(session: AgentSession) {
|
||||
let editorText = "";
|
||||
const editor: StubEditor = {
|
||||
setText(text) {
|
||||
editorText = text;
|
||||
},
|
||||
getText() {
|
||||
return editorText;
|
||||
},
|
||||
addToHistory: vi.fn(),
|
||||
};
|
||||
const pendingMessagesContainer = new Container();
|
||||
const requestRender = vi.fn();
|
||||
const updatePendingMessagesDisplay = vi.fn();
|
||||
|
||||
const ctx = {
|
||||
editor,
|
||||
ui: { requestRender },
|
||||
pendingMessagesContainer,
|
||||
session,
|
||||
compactionQueuedMessages: [],
|
||||
keybindings: {
|
||||
getDisplayString: (_action: string) => "Alt+Up",
|
||||
},
|
||||
updatePendingMessagesDisplay,
|
||||
locallySubmittedUserSignatures: new Set<string>(),
|
||||
} as unknown as InteractiveModeContext;
|
||||
|
||||
return { ctx, editor, pendingMessagesContainer };
|
||||
}
|
||||
|
||||
describe("UiHelpers / InputController against the queued-display layer (E8-E9)", () => {
|
||||
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!);
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
if (fixture) {
|
||||
await fixture.session.dispose();
|
||||
fixture.authStorage.close();
|
||||
fixture.tempDir.removeSync();
|
||||
fixture = undefined;
|
||||
}
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("E8: updatePendingMessagesDisplay renders the compact slash form for queued skills", async () => {
|
||||
fixture = await createRealSession();
|
||||
const { session } = fixture;
|
||||
session.enqueueCustomMessageDisplay("/skill:test-skill arg1 arg2", "steer");
|
||||
|
||||
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 () => {
|
||||
fixture = await createRealSession();
|
||||
const { session } = fixture;
|
||||
session.enqueueCustomMessageDisplay("/skill:test-skill arg1 arg2", "steer");
|
||||
|
||||
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([]);
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,137 @@
|
||||
/**
|
||||
* Phase 6 — F layer.
|
||||
*
|
||||
* Direct unit tests on:
|
||||
* - `SessionManager.appendCustomMessageEntry` — the single chokepoint that
|
||||
* routes `details` through `stripInternalDetailsFields` before persistence;
|
||||
* - `stripInternalDetailsFields` itself — the helper that enforces the
|
||||
* `INTERNAL_DETAILS_FIELDS` allowlist.
|
||||
*
|
||||
* The contract under test is the explicit-allowlist regression guard: only the
|
||||
* fields named in `INTERNAL_DETAILS_FIELDS` are removed; anything else (even
|
||||
* `__`-prefixed fields not in the allowlist) is preserved verbatim.
|
||||
*/
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import {
|
||||
type SkillPromptDetails,
|
||||
stripInternalDetailsFields,
|
||||
} from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { type CustomMessageEntry, SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
|
||||
const SKILL_TYPE = "skill-prompt";
|
||||
|
||||
function readPersistedCustomMessageEntry<T>(
|
||||
session: SessionManager,
|
||||
id: string,
|
||||
): CustomMessageEntry<T> {
|
||||
const branch = session.getBranch();
|
||||
const entry = branch.find(e => e.id === id);
|
||||
if (!entry || entry.type !== "custom_message") {
|
||||
throw new Error(`Expected custom_message entry with id ${id}, got ${entry?.type ?? "none"}`);
|
||||
}
|
||||
return entry as CustomMessageEntry<T>;
|
||||
}
|
||||
|
||||
describe("SessionManager.appendCustomMessageEntry (allowlist strip + persistence contract)", () => {
|
||||
it("F1: strips __pendingDisplayTag from persisted details while preserving all other SkillPromptDetails fields", () => {
|
||||
const session = SessionManager.inMemory();
|
||||
const id = session.appendCustomMessageEntry<SkillPromptDetails>(
|
||||
SKILL_TYPE,
|
||||
"skill body",
|
||||
true,
|
||||
{
|
||||
name: "foo",
|
||||
path: "/s.md",
|
||||
args: "bar",
|
||||
lineCount: 10,
|
||||
__pendingDisplayTag: "omp-cmd-1-0",
|
||||
},
|
||||
"user",
|
||||
);
|
||||
|
||||
const entry = readPersistedCustomMessageEntry<SkillPromptDetails>(session, id);
|
||||
expect(entry.details).toEqual({
|
||||
name: "foo",
|
||||
path: "/s.md",
|
||||
args: "bar",
|
||||
lineCount: 10,
|
||||
});
|
||||
// Explicit absence assertion — defends against `toEqual` semantics drift
|
||||
// where an `undefined`-valued key would still satisfy deep equality.
|
||||
expect(Object.hasOwn(entry.details!, "__pendingDisplayTag")).toBe(false);
|
||||
});
|
||||
|
||||
it("F2: persists details deep-equal to the input when no allowlisted field is present", () => {
|
||||
const session = SessionManager.inMemory();
|
||||
const input: SkillPromptDetails = {
|
||||
name: "foo",
|
||||
path: "/s.md",
|
||||
args: "bar",
|
||||
lineCount: 10,
|
||||
};
|
||||
const id = session.appendCustomMessageEntry<SkillPromptDetails>(SKILL_TYPE, "skill body", true, input, "user");
|
||||
const entry = readPersistedCustomMessageEntry<SkillPromptDetails>(session, id);
|
||||
// Deep equality on shape only — the contract intentionally does NOT couple
|
||||
// to whether the helper clones or short-circuits internally. Future
|
||||
// refactors (defensive cloning, JSON round-trip) cannot break this test.
|
||||
expect(entry.details).toEqual(input);
|
||||
});
|
||||
|
||||
it("F3: does NOT strip __-prefixed fields that are not in INTERNAL_DETAILS_FIELDS (explicit-allowlist guard)", () => {
|
||||
// Regression guard against an over-broad strip — only allowlisted keys go.
|
||||
// Future internal fields that haven't been added to the allowlist must be
|
||||
// preserved verbatim until that change ships intentionally.
|
||||
const session = SessionManager.inMemory();
|
||||
const id = session.appendCustomMessageEntry<Record<string, unknown>>(
|
||||
SKILL_TYPE,
|
||||
"skill body",
|
||||
true,
|
||||
{
|
||||
name: "foo",
|
||||
path: "/s.md",
|
||||
args: "bar",
|
||||
lineCount: 10,
|
||||
__future_field: "preserve-me",
|
||||
},
|
||||
"user",
|
||||
);
|
||||
const entry = readPersistedCustomMessageEntry<Record<string, unknown>>(session, id);
|
||||
expect(entry.details).toEqual({
|
||||
name: "foo",
|
||||
path: "/s.md",
|
||||
args: "bar",
|
||||
lineCount: 10,
|
||||
__future_field: "preserve-me",
|
||||
});
|
||||
});
|
||||
|
||||
it("F4: stripInternalDetailsFields treats undefined / null / non-object details as identity", () => {
|
||||
expect(stripInternalDetailsFields(undefined)).toBeUndefined();
|
||||
// `null as never` here only because the public signature is `T | undefined`,
|
||||
// but the runtime contract has to tolerate `null` defensively.
|
||||
expect(stripInternalDetailsFields(null as unknown as undefined)).toBeNull();
|
||||
expect(stripInternalDetailsFields("string" as unknown as undefined)).toBe(
|
||||
"string" as unknown as undefined,
|
||||
);
|
||||
});
|
||||
|
||||
it("F5: stripInternalDetailsFields preserves the input shape verbatim when no allowlisted field is present", () => {
|
||||
// Shape-preservation contract: the helper returns a value deep-equal to
|
||||
// the input when no allowlisted key is present. The plan's original
|
||||
// `Object.is` identity claim was deliberately weakened here to a
|
||||
// shape-preservation assertion so a future defensive-clone refactor
|
||||
// (e.g. structured-clone-on-read) cannot break this test without a real
|
||||
// behavioral regression. Identity / allocation strategy is an internal
|
||||
// implementation detail of the helper, not a public contract.
|
||||
const input = { name: "foo", lineCount: 1 };
|
||||
const result = stripInternalDetailsFields(input);
|
||||
expect(result).toEqual(input);
|
||||
// Every input key survives — no allowlisted field touched, so no key
|
||||
// dropped. Iterating the input's keys defends against a regression that
|
||||
// silently drops one even when the shape happens to match deep-equality
|
||||
// (e.g. via an extra `undefined` member).
|
||||
for (const key of Object.keys(input)) {
|
||||
expect(Object.hasOwn(result as object, key)).toBe(true);
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user