Merge PR #8979: fix(coding-agent): paint optimistic row for idle /skill submits (@roboomp)
This commit is contained in:
@@ -14,6 +14,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed the `/btw` panel re-committing its frame to native scrollback on every update while the primary turn is still streaming: a live region that pins itself (an anchored HUD/panel such as `/btw`) no longer leaks its scrolled-off rows just because an unpinned transcript seam sits above it in the same frame ([#8793](https://github.com/can1357/oh-my-pi/issues/8793)).
|
||||
- Fixed a submitted `/skill:<name>` command staying invisible in the transcript until its awaited preflight (memory recall, `before_agent_start` hooks, auto-thinking classification, pre-prompt compaction) finished, so a slow step such as a Hindsight auto-recall timeout made the command look unaccepted. Idle skill submissions now paint an optimistic row immediately — like a normal prompt — and reconcile it in place when the canonical `message_start` lands ([#8895](https://github.com/can1357/oh-my-pi/issues/8895)).
|
||||
- Fixed Claude Code marketplace plugins ignoring the `enabledPlugins` switch in `~/.claude/settings.json` and `.claude/settings(.local).json`: a plugin turned off for a project no longer loads there, and a local-scope install enabled for a project loads even when its recorded `projectPath` is a different directory
|
||||
- Fixed revived subagents (warm lifecycle reviver and cold persisted reviver) rebuilding the session without initializing the extension runtime, leaving every runtime action throwing `ExtensionRuntimeNotInitializedError`. An extension with a `tool_call` handler that touched a runtime action (e.g. `appendEntry`) then tripped the fail-closed gate in `emitToolCall` and blocked every tool — including the hidden `yield` — so the revived agent could neither finish nor exit and looped until killed. Both revivers now call the shared `initializeExtensions` helper, restoring runtime actions, `onError`, and the `session_start` event ([#8824](https://github.com/can1357/oh-my-pi/issues/8824)).
|
||||
- Fixed `omp commit` split-commit crashing with a misleading `No diff found for <path>` when a staged binary (or any payload) pushed `git diff --cached --binary` past the 8 MiB subprocess output cap. The capture is truncated silently, so files sorting after the binary vanished from the parsed diff; the split flow now requests a complete diff and fails fast naming the real cause instead ([#8897](https://github.com/can1357/oh-my-pi/issues/8897)).
|
||||
|
||||
@@ -24,7 +24,7 @@ import { getSymbolTheme, theme } from "../../modes/theme/theme";
|
||||
import type { InteractiveModeContext, TodoPhase } from "../../modes/types";
|
||||
import idleRecapPrompt from "../../prompts/system/recap-user.md" with { type: "text" };
|
||||
import type { AgentSessionEvent } from "../../session/agent-session";
|
||||
import { isSilentAbort, readQueueChipText, resolveAbortLabel } from "../../session/messages";
|
||||
import { isSilentAbort, isUserInvokedSkillPrompt, readQueueChipText, resolveAbortLabel } from "../../session/messages";
|
||||
import { type ApprovalMode, resolveApproval } from "../../tools/approval";
|
||||
import { previewLine, TRUNCATE_LENGTHS } from "../../tools/render-utils";
|
||||
import { PROPOSE_DEVICE_NAME, writeDeviceDispatch } from "../../tools/resolve";
|
||||
@@ -778,7 +778,17 @@ export class EventController {
|
||||
}
|
||||
this.#renderedCustomMessages.add(signature);
|
||||
this.#resetReadGroup();
|
||||
this.ctx.addMessageToChat(event.message);
|
||||
if (
|
||||
event.message.role === "custom" &&
|
||||
this.ctx.optimisticSkillMessagePending &&
|
||||
isUserInvokedSkillPrompt(event.message)
|
||||
) {
|
||||
// The optimistic `/skill:` row painted at submit time (issue #8895):
|
||||
// swap it for the canonical message instead of appending a duplicate.
|
||||
this.ctx.reconcileOptimisticSkillMessage(event.message);
|
||||
} else {
|
||||
this.ctx.addMessageToChat(event.message);
|
||||
}
|
||||
// Queued custom-message chips are derived from the agent queue; refresh the
|
||||
// pending bar when the queued custom is consumed so the chip disappears
|
||||
// immediately.
|
||||
|
||||
@@ -17,7 +17,7 @@ import { expandEmoticons } from "../../modes/emoji-autocomplete";
|
||||
import { materializeImageReferenceLinks, shiftImageMarkers } from "../../modes/image-references";
|
||||
import { createPromptActionAutocompleteProvider } from "../../modes/prompt-action-autocomplete";
|
||||
import { parseQueueShorthand, splitQueuedMessages } from "../../modes/queue-input";
|
||||
import { invokeSkillCommandFromText, isKnownSkillCommand } from "../../modes/skill-command";
|
||||
import { buildSkillCommandPrompt, isKnownSkillCommand } from "../../modes/skill-command";
|
||||
import type { InteractiveModeContext } from "../../modes/types";
|
||||
import manualContinuePrompt from "../../prompts/system/manual-continue.md" with { type: "text" };
|
||||
import { USER_INTERRUPT_LABEL } from "../../session/messages";
|
||||
@@ -1173,21 +1173,42 @@ export class InputController {
|
||||
};
|
||||
|
||||
this.ctx.editor.clearDraft(text);
|
||||
let optimistic = false;
|
||||
try {
|
||||
const handled = await invokeSkillCommandFromText(this.ctx, text, streamingBehavior, {
|
||||
images: draftImages,
|
||||
propagateErrors: true,
|
||||
});
|
||||
if (!handled) {
|
||||
// Build the user-attributed skill message once so the optimistic
|
||||
// transcript row and the dispatched message share content.
|
||||
const built = await buildSkillCommandPrompt(this.ctx, text, streamingBehavior, draftImages);
|
||||
if (!built) {
|
||||
restoreDraft();
|
||||
return false;
|
||||
}
|
||||
// Paint the row before the awaited dispatch so a slow preflight (memory
|
||||
// recall, before_agent_start hooks, auto-thinking, pre-prompt compaction)
|
||||
// does not leave the submission invisible (issue #8895). A streaming
|
||||
// submission queues instead and surfaces its chip, so only paint when the
|
||||
// turn will run fresh.
|
||||
optimistic = !this.ctx.session.isStreaming;
|
||||
if (optimistic) {
|
||||
// Mirror the message promptCustomMessage will build for the turn so the
|
||||
// canonical message_start reconciles this row rather than duplicating it.
|
||||
this.ctx.renderOptimisticSkillMessage(
|
||||
{ role: "custom", ...built.message, timestamp: Date.now() },
|
||||
{ imageLinks: draftImageLinks },
|
||||
);
|
||||
}
|
||||
await this.ctx.session.promptCustomMessage(built.message, built.options);
|
||||
return true;
|
||||
} catch (error) {
|
||||
if (optimistic) this.ctx.clearOptimisticSkillMessage();
|
||||
restoreDraft();
|
||||
this.ctx.showError(error instanceof Error ? error.message : String(error));
|
||||
return true;
|
||||
} finally {
|
||||
if (optimistic && this.ctx.optimisticSkillMessagePending) {
|
||||
// Dispatch resolved without a canonical skill message_start (aborted
|
||||
// preflight, or a streaming-race requeue): drop the pending row.
|
||||
this.ctx.clearOptimisticSkillMessage();
|
||||
}
|
||||
if (this.ctx.session.isStreaming) {
|
||||
this.ctx.updatePendingMessagesDisplay();
|
||||
this.ctx.ui.requestRender();
|
||||
|
||||
@@ -612,6 +612,10 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
#pendingSubmissionDispose: (() => void) | undefined;
|
||||
#pendingSubmissionPreservesDraft = false;
|
||||
#optimisticUserMessageComponents: Component[] = [];
|
||||
#optimisticSkillMessageComponents: Component[] = [];
|
||||
/** True while an optimistically-rendered `/skill:` row awaits its canonical
|
||||
* `message_start`. Read by the event controller to reconcile the row. */
|
||||
optimisticSkillMessagePending = false;
|
||||
lastSigintTime = 0;
|
||||
lastEscapeTime = 0;
|
||||
lastLeftTapTime = 0;
|
||||
@@ -1689,6 +1693,51 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
this.addMessageToChat(message, options);
|
||||
}
|
||||
|
||||
/**
|
||||
* Optimistically render a user-invoked `/skill:` row before its awaited
|
||||
* dispatch so a slow preflight (memory recall, `before_agent_start` hooks,
|
||||
* auto-thinking classification, pre-prompt compaction) does not leave the
|
||||
* submission invisible — normal prompts paint their row via
|
||||
* {@link startPendingSubmission} the same way (issue #8895). The canonical
|
||||
* skill `message_start` swaps this row in place via
|
||||
* {@link reconcileOptimisticSkillMessage}; a failed or bailed dispatch drops
|
||||
* it via {@link clearOptimisticSkillMessage}.
|
||||
*/
|
||||
renderOptimisticSkillMessage(
|
||||
message: AgentMessage,
|
||||
options?: { imageLinks?: readonly (string | undefined)[] },
|
||||
): void {
|
||||
this.clearOptimisticSkillMessage();
|
||||
this.optimisticSkillMessagePending = true;
|
||||
this.#optimisticSkillMessageComponents = this.#captureAddedChatComponents(() => {
|
||||
this.addMessageToChat(message, options);
|
||||
});
|
||||
this.ensureLoadingAnimation();
|
||||
this.ui.requestRender();
|
||||
}
|
||||
|
||||
/** Replace the optimistic `/skill:` row with the canonical message emitted by
|
||||
* the session, mirroring {@link replaceOptimisticUserMessage} for skills. */
|
||||
reconcileOptimisticSkillMessage(message: AgentMessage): void {
|
||||
this.optimisticSkillMessagePending = false;
|
||||
for (const component of this.#optimisticSkillMessageComponents) {
|
||||
this.chatContainer.removeChild(component);
|
||||
}
|
||||
this.#optimisticSkillMessageComponents = [];
|
||||
this.addMessageToChat(message);
|
||||
}
|
||||
|
||||
/** Drop the optimistic `/skill:` row when dispatch fails or bails before the
|
||||
* message reaches the agent (aborted preflight, streaming-race requeue). */
|
||||
clearOptimisticSkillMessage(): void {
|
||||
this.optimisticSkillMessagePending = false;
|
||||
if (this.#optimisticSkillMessageComponents.length === 0) return;
|
||||
for (const component of this.#optimisticSkillMessageComponents) {
|
||||
this.chatContainer.removeChild(component);
|
||||
}
|
||||
this.#optimisticSkillMessageComponents = [];
|
||||
}
|
||||
|
||||
startPendingSubmission(
|
||||
input: {
|
||||
text: string;
|
||||
|
||||
@@ -313,6 +313,17 @@ export interface InteractiveModeContext {
|
||||
message: AgentMessage,
|
||||
options?: { imageLinks?: readonly (string | undefined)[] },
|
||||
): void;
|
||||
/** True while an optimistically-rendered `/skill:` row awaits its canonical `message_start`. */
|
||||
optimisticSkillMessagePending: boolean;
|
||||
/** Optimistically renders a user-invoked `/skill:` row before its awaited dispatch (issue #8895). */
|
||||
renderOptimisticSkillMessage(
|
||||
message: AgentMessage,
|
||||
options?: { imageLinks?: readonly (string | undefined)[] },
|
||||
): void;
|
||||
/** Swaps the optimistic `/skill:` row for the canonical message emitted by the session. */
|
||||
reconcileOptimisticSkillMessage(message: AgentMessage): void;
|
||||
/** Drops the optimistic `/skill:` row when dispatch fails or bails before reaching the agent. */
|
||||
clearOptimisticSkillMessage(): void;
|
||||
isKnownSlashCommand(text: string): boolean;
|
||||
addMessageToChat(
|
||||
message: AgentMessage,
|
||||
|
||||
@@ -89,6 +89,9 @@ function createStubInputControllerContext(opts: {
|
||||
const updatePendingMessagesDisplay = vi.fn();
|
||||
const requestRender = vi.fn();
|
||||
const showError = vi.fn();
|
||||
const renderOptimisticSkillMessage = vi.fn();
|
||||
const reconcileOptimisticSkillMessage = vi.fn();
|
||||
const clearOptimisticSkillMessage = vi.fn();
|
||||
const queueCompactionMessage = vi.fn((_text: string, _mode: "steer" | "followUp", _images?: ImageContent[]) => {});
|
||||
const ctx = {
|
||||
editor,
|
||||
@@ -117,6 +120,10 @@ function createStubInputControllerContext(opts: {
|
||||
locallySubmittedUserSignatures: new Set<string>(),
|
||||
withLocalSubmission: async (_text: string, fn: () => unknown) => fn(),
|
||||
queueCompactionMessage,
|
||||
optimisticSkillMessagePending: false,
|
||||
renderOptimisticSkillMessage,
|
||||
reconcileOptimisticSkillMessage,
|
||||
clearOptimisticSkillMessage,
|
||||
} as unknown as InteractiveModeContext;
|
||||
|
||||
return {
|
||||
@@ -128,6 +135,10 @@ function createStubInputControllerContext(opts: {
|
||||
updatePendingMessagesDisplay,
|
||||
requestRender,
|
||||
queueCompactionMessage,
|
||||
showError,
|
||||
renderOptimisticSkillMessage,
|
||||
reconcileOptimisticSkillMessage,
|
||||
clearOptimisticSkillMessage,
|
||||
};
|
||||
}
|
||||
|
||||
@@ -259,6 +270,83 @@ describe("InputController skill queue chip metadata", () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe("InputController optimistic skill row (#8895)", () => {
|
||||
let tempDir: TempDir;
|
||||
let skillCommands: Map<string, Skill>;
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = TempDir.createSync("@pi-skill-optimistic-stub-");
|
||||
const skill = await writeSkillFile(tempDir.path(), "test-skill", "Do the thing.");
|
||||
skillCommands = new Map<string, Skill>([["skill:test-skill", skill]]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
tempDir.removeSync();
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("paints the optimistic row before dispatching the idle skill turn", async () => {
|
||||
const { ctx, editor, promptCustomMessage, renderOptimisticSkillMessage } = createStubInputControllerContext({
|
||||
skillCommands,
|
||||
isStreaming: false,
|
||||
});
|
||||
// A slow preflight (memory recall, before_agent_start hooks, auto-thinking)
|
||||
// lives inside promptCustomMessage; the row must already be painted when the
|
||||
// dispatch begins, so it stays visible while that preflight runs.
|
||||
const order: string[] = [];
|
||||
renderOptimisticSkillMessage.mockImplementation(() => order.push("render"));
|
||||
promptCustomMessage.mockImplementation(async () => {
|
||||
order.push("dispatch");
|
||||
});
|
||||
|
||||
const controller = new InputController(ctx);
|
||||
controller.setupEditorSubmitHandler();
|
||||
editor.setText("/skill:test-skill go");
|
||||
await editor.onSubmit?.("/skill:test-skill go");
|
||||
|
||||
expect(order).toEqual(["render", "dispatch"]);
|
||||
expect(renderOptimisticSkillMessage.mock.calls[0]?.[0]).toMatchObject({
|
||||
role: "custom",
|
||||
customType: SKILL_PROMPT_MESSAGE_TYPE,
|
||||
attribution: "user",
|
||||
display: true,
|
||||
details: { name: "test-skill" },
|
||||
});
|
||||
});
|
||||
|
||||
it("skips the optimistic row when the skill submission queues while streaming", async () => {
|
||||
const { ctx, editor, promptCustomMessage, renderOptimisticSkillMessage } = createStubInputControllerContext({
|
||||
skillCommands,
|
||||
isStreaming: true,
|
||||
});
|
||||
const controller = new InputController(ctx);
|
||||
controller.setupEditorSubmitHandler();
|
||||
editor.setText("/skill:test-skill go");
|
||||
await editor.onSubmit?.("/skill:test-skill go");
|
||||
|
||||
expect(renderOptimisticSkillMessage).not.toHaveBeenCalled();
|
||||
expect(promptCustomMessage).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it("drops the optimistic row and restores the draft when dispatch throws", async () => {
|
||||
const { ctx, editor, promptCustomMessage, renderOptimisticSkillMessage, clearOptimisticSkillMessage, showError } =
|
||||
createStubInputControllerContext({ skillCommands, isStreaming: false });
|
||||
promptCustomMessage.mockImplementation(async () => {
|
||||
throw new Error("preflight failed");
|
||||
});
|
||||
|
||||
const controller = new InputController(ctx);
|
||||
controller.setupEditorSubmitHandler();
|
||||
editor.setText("/skill:test-skill go");
|
||||
await editor.onSubmit?.("/skill:test-skill go");
|
||||
|
||||
expect(renderOptimisticSkillMessage).toHaveBeenCalledTimes(1);
|
||||
expect(clearOptimisticSkillMessage).toHaveBeenCalledTimes(1);
|
||||
expect(showError).toHaveBeenCalledTimes(1);
|
||||
expect(editor.getText()).toBe("/skill:test-skill go");
|
||||
});
|
||||
});
|
||||
|
||||
describe("compaction skill re-invocation", () => {
|
||||
let tempDir: TempDir;
|
||||
let skillCommands: Map<string, Skill>;
|
||||
@@ -761,10 +849,11 @@ describe("UiHelpers / InputController against derived queued custom display", ()
|
||||
});
|
||||
});
|
||||
|
||||
function createEventControllerFixture() {
|
||||
function createEventControllerFixture(opts?: { optimisticSkillMessagePending?: boolean }) {
|
||||
const updatePendingMessagesDisplay = vi.fn();
|
||||
const addMessageToChat = vi.fn();
|
||||
const requestRender = vi.fn();
|
||||
const reconcileOptimisticSkillMessage = vi.fn();
|
||||
const ctx = {
|
||||
isInitialized: true,
|
||||
init: vi.fn(async () => {}),
|
||||
@@ -776,13 +865,15 @@ function createEventControllerFixture() {
|
||||
transcriptMessageComponents: new WeakMap(),
|
||||
pendingTools: new Map(),
|
||||
session: {},
|
||||
optimisticSkillMessagePending: opts?.optimisticSkillMessagePending ?? false,
|
||||
reconcileOptimisticSkillMessage,
|
||||
get viewSession() {
|
||||
return (this as typeof ctx).session;
|
||||
},
|
||||
} as unknown as InteractiveModeContext;
|
||||
|
||||
const controller = new EventController(ctx);
|
||||
return { controller, updatePendingMessagesDisplay, addMessageToChat };
|
||||
return { controller, updatePendingMessagesDisplay, addMessageToChat, reconcileOptimisticSkillMessage };
|
||||
}
|
||||
|
||||
describe("EventController custom queued-message refresh", () => {
|
||||
@@ -829,4 +920,49 @@ describe("EventController custom queued-message refresh", () => {
|
||||
expect(updatePendingMessagesDisplay).toHaveBeenCalledTimes(1);
|
||||
expect(addMessageToChat).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
it("reconciles the optimistic skill row instead of duplicating it on the canonical message_start", async () => {
|
||||
const { controller, addMessageToChat, reconcileOptimisticSkillMessage } = createEventControllerFixture({
|
||||
optimisticSkillMessagePending: true,
|
||||
});
|
||||
const event: Extract<AgentSessionEvent, { type: "message_start" }> = {
|
||||
type: "message_start",
|
||||
message: {
|
||||
role: "custom",
|
||||
customType: SKILL_PROMPT_MESSAGE_TYPE,
|
||||
content: "skill body",
|
||||
display: true,
|
||||
attribution: "user",
|
||||
details: { name: "test-skill", path: "/s.md", lineCount: 1 } satisfies SkillPromptDetails,
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
};
|
||||
await controller.handleEvent(event);
|
||||
|
||||
expect(reconcileOptimisticSkillMessage).toHaveBeenCalledTimes(1);
|
||||
expect(reconcileOptimisticSkillMessage.mock.calls[0]?.[0]).toBe(event.message);
|
||||
expect(addMessageToChat).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("renders normally when no optimistic skill row is pending", async () => {
|
||||
const { controller, addMessageToChat, reconcileOptimisticSkillMessage } = createEventControllerFixture({
|
||||
optimisticSkillMessagePending: false,
|
||||
});
|
||||
const event: Extract<AgentSessionEvent, { type: "message_start" }> = {
|
||||
type: "message_start",
|
||||
message: {
|
||||
role: "custom",
|
||||
customType: SKILL_PROMPT_MESSAGE_TYPE,
|
||||
content: "skill body",
|
||||
display: true,
|
||||
attribution: "user",
|
||||
details: { name: "test-skill", path: "/s.md", lineCount: 1 } satisfies SkillPromptDetails,
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
};
|
||||
await controller.handleEvent(event);
|
||||
|
||||
expect(reconcileOptimisticSkillMessage).not.toHaveBeenCalled();
|
||||
expect(addMessageToChat).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user