fix(tui): render mid-turn steering skips as info, not errors

A tool call aborted mid-batch to service queued steering/peer input emits a synthetic placeholder result with isError:true so the model retries it. The TUI keyed all error styling (red ✘, red frame/text) off that flag, so a normal steering skip rendered identically to a real tool failure.

Mark the skip placeholder with the existing SyntheticToolResultDetails discriminator (source: "interrupt_skipped", executed:false) and render benign skips through the neutral generic card (info glyph, dim text, neutral background), bypassing any bespoke error frame. Genuine failures keep their error styling.

Fixes #7199
This commit is contained in:
roboomp
2026-07-31 21:06:07 +00:00
parent 80627462b4
commit ebd84d4f85
6 changed files with 171 additions and 30 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Tool calls skipped mid-batch to service a queued steering/peer interrupt now carry the `SyntheticToolResultDetails` discriminator (`source: "interrupt_skipped"`, `executed: false`), so UI/telemetry consumers can classify them as "call emitted, not executed" instead of a real tool failure ([#7199](https://github.com/can1357/oh-my-pi/issues/7199)).
## [17.2.2] - 2026-07-31
### Fixed
+15 -6
View File
@@ -2723,13 +2723,20 @@ async function executeToolCalls(
* (#4321): a provider-side stream error after tool-call emission (e.g. Codex
* websocket close) was surfaced by the CLI as if the local tool had failed.
*
* `source` names the assistant-side termination state that prevented
* execution; `upstreamError` is the provider-reported message when the turn
* ended with `stopReason === "error"`.
* `source` names the state that prevented execution — either an assistant-side
* turn termination (`assistant_stop_*`) or a mid-batch interrupt that skipped a
* still-pending call to service queued steering/peer input (`interrupt_skipped`).
* `upstreamError` is the provider-reported message when the turn ended with
* `stopReason === "error"`.
*/
export interface SyntheticToolResultDetails {
__synthetic: true;
source: "assistant_stop_aborted" | "assistant_stop_error" | "assistant_stop_skipped" | "assistant_stop_length";
source:
| "assistant_stop_aborted"
| "assistant_stop_error"
| "assistant_stop_skipped"
| "assistant_stop_length"
| "interrupt_skipped";
executed: false;
upstreamError?: string;
}
@@ -2844,7 +2851,9 @@ function createToolSignalAbortedResult(signal: AbortSignal): AgentToolResult<unk
};
}
function createSkippedToolResult(source: SteeringInterruptSource | "irc" | undefined): AgentToolResult<any> {
function createSkippedToolResult(
source: SteeringInterruptSource | "irc" | undefined,
): AgentToolResult<SyntheticToolResultDetails> {
let reason = "pending steering message";
let blocker = "queued message";
if (source === "user") {
@@ -2864,6 +2873,6 @@ function createSkippedToolResult(source: SteeringInterruptSource | "irc" | undef
text: `Skipped due to ${reason}. Do not count this skipped result as completed work or verification. After the ${blocker} is handled on the next step, retry the skipped tool if it is still needed.`,
},
],
details: {},
details: { __synthetic: true, source: "interrupt_skipped", executed: false },
};
}
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed mid-turn steering/peer-interrupt tool skips rendering as errors (red ✘, red border/text) in the TUI; the synthetic skip placeholder now renders as a neutral info card, matching its "call emitted, not executed" meaning ([#7199](https://github.com/can1357/oh-my-pi/issues/7199)).
## [17.2.2] - 2026-07-31
### Added
@@ -276,6 +276,9 @@ let toolExecutionInstanceSeq = 0;
export class ToolExecutionComponent extends Container implements NativeScrollbackLiveRegion {
#contentBox: Box; // Used for custom tools and bash visual truncation
#contentText: WidthAwareText; // Generic fallback (no custom/built-in renderer)
// Which container the constructor mounted: bespoke/built-in renderers use
// #contentBox, everything else the generic #contentText fallback.
#usesContentBox = false;
#multiFileBoxes: (Box | Spacer)[] = []; // Extra boxes for multi-file edit results
#imageComponents: Image[] = [];
#imageSpacers: Spacer[] = [];
@@ -411,26 +414,13 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
// strips PLAIN-blank edges, so framed/minimal blocks (no bg set) drop these
// lines and keep their tight spacing — only tinted lines survive.
this.#contentBox = new Box(0, 1);
this.#contentText = new WidthAwareText(
contentWidth =>
formatDefaultToolExecution(
{
label: this.#toolLabel,
args: this.#args,
result: this.#result ? { output: this.#getTextOutput(), isError: this.#result.isError } : undefined,
options: this.#renderState,
},
contentWidth,
theme,
),
1,
1,
);
this.#contentText = new WidthAwareText(contentWidth => this.#renderDefaultCard(contentWidth), 1, 1);
// Use Box for custom tools or built-in tools that have renderers
const hasRenderer = toolName in toolRenderers;
const hasCustomRenderer = !!(tool?.renderCall || tool?.renderResult);
if (hasCustomRenderer || hasRenderer) {
this.#usesContentBox = hasCustomRenderer || hasRenderer;
if (this.#usesContentBox) {
this.addChild(this.#contentBox);
} else {
this.addChild(this.#contentText);
@@ -998,12 +988,21 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
// Non-self-framing tools (custom/extension renderers and the generic
// fallback) get a padded, state-tinted block — built-ins that draw their
// own frame opt out below via the framed-component mark.
const stateBgKey = this.#isPartial ? "toolPendingBg" : this.#result?.isError ? "toolErrorBg" : "toolSuccessBg";
// own frame opt out below via the framed-component mark. A benign skip
// (steering/peer interrupt aborted a still-pending call) never ran, so it
// gets the neutral pending tint rather than the error tint (#7199).
const benignSkip = this.#isBenignSkip();
const stateBgKey =
this.#isPartial || benignSkip ? "toolPendingBg" : this.#result?.isError ? "toolErrorBg" : "toolSuccessBg";
const stateBgFn = (t: string) => theme.bg(stateBgKey, t);
// Check for custom tool rendering
if (this.#tool && (this.#tool.renderCall || this.#tool.renderResult)) {
// A benign skip is a synthetic placeholder for a call that never executed,
// so bypass any bespoke error frame and draw the neutral generic card —
// the per-tool ✘/red-border would misread normal mid-turn steering as a
// failure (#7199).
if (benignSkip) {
this.#renderBenignSkipCard(stateBgFn);
} else if (this.#tool && (this.#tool.renderCall || this.#tool.renderResult)) {
const tool = this.#tool;
const mergeCallAndResult = Boolean((tool as { mergeCallAndResult?: boolean }).mergeCallAndResult);
// Custom tools use Box for flexible component rendering
@@ -1399,4 +1398,58 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
return output;
}
/**
* Format the generic call/result card at `contentWidth`. Shared by the
* #contentText fallback and the benign-skip path so both render identically.
*/
#renderDefaultCard(contentWidth: number): string {
return formatDefaultToolExecution(
{
label: this.#toolLabel,
args: this.#args,
result: this.#result
? { output: this.#getTextOutput(), isError: this.#result.isError, skipped: this.#isBenignSkip() }
: undefined,
options: this.#renderState,
},
contentWidth,
theme,
);
}
/**
* True when the settled result is the synthetic placeholder emitted for a
* tool call skipped mid-batch to service queued steering/peer input. Such a
* call never executed, so it must not render as an error (#7199).
*/
#isBenignSkip(): boolean {
if (this.#isPartial || !this.#result) return false;
const details = this.#result.details as { __synthetic?: boolean; source?: string } | undefined;
return details?.__synthetic === true && details.source === "interrupt_skipped";
}
/**
* Render a benign skip as the neutral generic card, replacing any bespoke
* renderer's error frame. Generic-fallback tools already route through
* {@link #renderDefaultCard} (which emits the info card for a skip); they
* only need the neutral tint. Bespoke-renderer tools get their content box
* swapped for the same neutral card.
*/
#renderBenignSkipCard(stateBgFn: (text: string) => string): void {
if (!this.#usesContentBox) {
this.#contentText.setCustomBgFn(stateBgFn);
this.#contentText.invalidate();
return;
}
for (const box of this.#multiFileBoxes) {
this.removeChild(box);
}
this.#multiFileBoxes = [];
this.#contentBox.setBgFn(undefined);
this.#contentBox.clear();
this.#contentBox.setPaddingX(1);
this.#contentBox.setBgFn(stateBgFn);
this.#contentBox.addChild(new WidthAwareText(contentWidth => this.#renderDefaultCard(contentWidth), 0, 0));
}
}
@@ -25,6 +25,9 @@ export interface DefaultToolRenderInput {
result?: {
output: string;
isError?: boolean;
/** Synthetic placeholder for a call skipped mid-batch to service steering/peer
* input — the tool never ran, so it renders neutral (info) rather than as an error. */
skipped?: boolean;
};
/** Current expansion and lifecycle state. */
options: RenderResultOptions;
@@ -42,10 +45,22 @@ export function formatDefaultToolExecution(
? options.spinnerFrame !== undefined
? "running"
: "pending"
: result?.isError
? "error"
: "done";
lines.push(renderStatusLine({ icon, spinnerFrame: options.spinnerFrame, title: input.label }, uiTheme));
: result?.skipped
? "info"
: result?.isError
? "error"
: "done";
lines.push(
renderStatusLine(
{
icon,
spinnerFrame: options.spinnerFrame,
title: input.label,
...(result?.skipped ? { titleColor: "muted" as const } : {}),
},
uiTheme,
),
);
const args = isRecord(input.args) ? input.args : undefined;
if (!options.expanded && args && Object.keys(args).length > 0) {
@@ -0,0 +1,56 @@
import { beforeAll, describe, expect, it } from "bun:test";
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import { ToolExecutionComponent } from "@oh-my-pi/pi-coding-agent/modes/components/tool-execution";
import { getThemeByName, initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
import { formatStatusIcon } from "@oh-my-pi/pi-coding-agent/tools/render-utils";
import { TUI } from "@oh-my-pi/pi-tui";
import { VirtualTerminal } from "../../tui/test/virtual-terminal";
beforeAll(async () => {
resetSettingsForTest();
await Settings.init({ inMemory: true, cwd: process.cwd() });
await initTheme(false, undefined, undefined, "dark", "light");
}, 15_000);
const SKIP_TEXT =
"Skipped due to pending peer interrupt. Do not count this skipped result as completed work or verification. After the interrupt is handled on the next step, retry the skipped tool if it is still needed.";
function renderSkippedEdit(details: unknown): string {
const tui = new TUI(new VirtualTerminal(120, 20));
const component = new ToolExecutionComponent("edit", { path: "hub/src/viewer/session.ts" }, {}, undefined, tui);
component.updateResult({ content: [{ type: "text", text: SKIP_TEXT }], details, isError: true }, false);
return Bun.stripANSI(component.render(120).join("\n"));
}
describe("mid-turn steering skip rendering", () => {
it("renders a synthetic interrupt-skip as info, not an error", async () => {
const uiTheme = await getThemeByName("dark");
if (!uiTheme) throw new Error("dark theme missing");
const errorIcon = Bun.stripANSI(formatStatusIcon("error", uiTheme));
const infoIcon = Bun.stripANSI(formatStatusIcon("info", uiTheme));
const rendered = renderSkippedEdit({
__synthetic: true,
source: "interrupt_skipped",
executed: false,
});
expect(rendered).toContain(infoIcon);
expect(rendered).not.toContain(errorIcon);
// The bespoke edit error frame must be gone — a skip is not a failure.
expect(rendered).not.toContain("╭");
expect(rendered).toContain("Skipped due to pending peer interrupt");
}, 15_000);
it("still renders a genuine edit failure as an error", async () => {
const uiTheme = await getThemeByName("dark");
if (!uiTheme) throw new Error("dark theme missing");
const errorIcon = Bun.stripANSI(formatStatusIcon("error", uiTheme));
// A real tool failure carries no synthetic discriminator and must keep the
// error styling — the fix only neutralizes benign interrupt skips.
const rendered = renderSkippedEdit({});
expect(rendered).toContain(errorIcon);
}, 15_000);
});