smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(agent): distinguished started steering aborts

Browse Source 
Track entry into tool.execute separately from tool event emission. Never-started skips retain SyntheticToolResultDetails with executed:false; in-flight aborts now use distinct interrupted metadata with execution:started so consumers do not assume no partial work occurred.

Keep both interrupt states neutral in the TUI and cover the agent metadata boundary plus rendering behavior.

Fixes #7199
This commit is contained in:
roboomp
2026-07-31 21:16:28 +00:00
parent ebd84d4f85
commit af343f70d0
6 changed files with 112 additions and 26 deletions
Download Patch File Download Diff File Expand all files Collapse all files
packages/agent/CHANGELOG.md
+1 -1
View File
@@ -4,7 +4,7 @@
### 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)).
- Tool calls skipped mid-batch to service queued steering/peer input now distinguish calls that never entered `tool.execute` (`SyntheticToolResultDetails`, `executed: false`) from in-flight calls that may have performed partial work (`execution: "started"`), allowing UI/telemetry consumers to render normal steering control flow without misreporting execution state ([#7199](https://github.com/can1357/oh-my-pi/issues/7199)).
## [17.2.2] - 2026-07-31
packages/agent/src/agent-loop.ts
+23 -8
View File
@@ -2454,6 +2454,7 @@ async function executeToolCalls(
let isError = false;
let caughtError: unknown;
let completedToolExecution = false;
let executionStarted = false;
await runInActiveSpan(toolSpan, async () => {
try {
@@ -2487,6 +2488,7 @@ async function executeToolCalls(
providerMetadata: toolCall.providerMetadata,
})
: undefined;
executionStarted = true;
const rawResult = await tool.execute(
toolCall.id,
executionArgs,
@@ -2557,12 +2559,12 @@ async function executeToolCalls(
const interrupted = interruptState.triggered;
const perToolAborted = record.signal.aborted;
const abortedDuringExecution = perToolAborted && isError && !completedToolExecution;
if (interrupted && perToolAborted && isError && !completedToolExecution) {
// This tool's own signal fired AND it failed to produce a result: `tool.execute()`
// never returned (it threw on the abort), so it was genuinely cut off before
// producing usable output. Report it as skipped.
if (interrupted && abortedDuringExecution) {
// This tool's own signal fired AND it failed to produce a result. The
// execution may already have performed partial work before throwing on
// abort, so preserve that distinction in the placeholder metadata.
record.skipped = true;
emitToolResult(record, createSkippedToolResult(interruptState.source), true);
emitToolResult(record, createSkippedToolResult(interruptState.source, executionStarted), true);
} else {
// No interrupt on this signal, or the tool finished before the interrupt landed
// (`completedToolExecution`) — even if the signal aborted around completion. Keep
@@ -2703,7 +2705,7 @@ async function executeToolCalls(
toolName: record.toolCall.name,
status: "skipped",
});
emitToolResult(record, createSkippedToolResult(interruptState.source), true);
emitToolResult(record, createSkippedToolResult(interruptState.source, false), true);
}
}
@@ -2741,6 +2743,16 @@ export interface SyntheticToolResultDetails {
upstreamError?: string;
}
/**
* Metadata for an interrupt-aborted call that entered `tool.execute()` but
* threw before returning a usable result. It may have performed partial work.
*/
interface InterruptedToolResultDetails {
__interrupted: true;
source: "interrupt_skipped";
execution: "started";
}
/**
* Narrow an {@link AgentMessage} to a synthetic {@link ToolResultMessage} —
* a tool_result emitted for a tool call the assistant never invoked (see
@@ -2853,7 +2865,8 @@ function createToolSignalAbortedResult(signal: AbortSignal): AgentToolResult<unk
function createSkippedToolResult(
source: SteeringInterruptSource | "irc" | undefined,
): AgentToolResult<SyntheticToolResultDetails> {
executionStarted: boolean,
): AgentToolResult<SyntheticToolResultDetails | InterruptedToolResultDetails> {
let reason = "pending steering message";
let blocker = "queued message";
if (source === "user") {
@@ -2873,6 +2886,8 @@ function createSkippedToolResult(
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: { __synthetic: true, source: "interrupt_skipped", executed: false },
details: executionStarted
? { __interrupted: true, source: "interrupt_skipped", execution: "started" }
: { __synthetic: true, source: "interrupt_skipped", executed: false },
};
}
packages/agent/test/agent-loop.test.ts
+65
View File
@@ -1500,6 +1500,11 @@ describe("agentLoop with AgentMessage", () => {
expect(toolEnds.length).toBe(2);
expect(toolEnds[0].isError).toBe(false);
expect(toolEnds[1].isError).toBe(true);
expect(toolEnds[1].result.details).toEqual({
__synthetic: true,
source: "interrupt_skipped",
executed: false,
});
const skippedContent = toolEnds[1].result.content[0];
expect(skippedContent?.type).toBe("text");
if (skippedContent?.type !== "text") throw new Error("skipped tool result must be text");
@@ -1692,6 +1697,66 @@ describe("agentLoop with AgentMessage", () => {
).toBe(true);
});
it("distinguishes an in-flight abort from a never-executed steering skip", async () => {
const toolSchema = type({});
let steerReady = false;
let drained = false;
const tool: AgentTool<typeof toolSchema, Record<string, never>> = {
name: "wait",
label: "Wait",
description: "Starts work, then throws when steering aborts it",
parameters: toolSchema,
interruptible: true,
async execute(_toolCallId, _params, signal) {
steerReady = true;
if (!signal) throw new Error("missing tool abort signal");
const aborted = Promise.withResolvers<void>();
if (signal.aborted) {
aborted.resolve();
} else {
signal.addEventListener("abort", () => aborted.resolve(), { once: true });
}
await aborted.promise;
throw new Error("aborted after partial work");
},
};
const context: AgentContext = { systemPrompt: [""], messages: [], tools: [tool] };
const mock = createMockModel({
responses: [
{ content: [{ type: "toolCall", id: "tool-1", name: "wait", arguments: {} }] },
{ content: ["done"] },
],
});
const config: AgentLoopConfig = {
model: mock.model,
convertToLlm: identityConverter,
interruptMode: "immediate",
hasSteeringMessages: () => steerReady && !drained,
getSteeringMessages: async () => {
if (!steerReady || drained) return [];
drained = true;
return [createUserMessage("interrupt")];
},
};
const events: AgentEvent[] = [];
for await (const event of agentLoop([createUserMessage("start")], context, config, undefined, mock.stream)) {
events.push(event);
}
const toolEnd = events.find(
(event): event is Extract<AgentEvent, { type: "tool_execution_end" }> => event.type === "tool_execution_end",
);
expect(toolEnd?.result.details).toEqual({
__interrupted: true,
source: "interrupt_skipped",
execution: "started",
});
expect(toolEnd?.result.details).not.toHaveProperty("executed");
});
it("keeps a completed error result instead of clobbering it into skipped when a steer aborts the signal (#4752)", async () => {
// A steer lands while an interruptible tool is in flight, aborting its shared
// signal via the mid-batch watch poll. The tool nonetheless runs to completion
packages/coding-agent/CHANGELOG.md
+1 -1
View File
@@ -4,7 +4,7 @@
### 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)).
- Fixed mid-turn steering/peer-interrupt tool skips rendering as errors (red ✘, red border/text) in the TUI; pending and in-flight interrupt placeholders now render as neutral info cards while preserving whether `tool.execute` started ([#7199](https://github.com/can1357/oh-my-pi/issues/7199)).
## [17.2.2] - 2026-07-31
packages/coding-agent/src/modes/components/tool-execution.ts
+9 -5
View File
@@ -1419,14 +1419,18 @@ export class ToolExecutionComponent extends Container implements NativeScrollbac
}
/**
* 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).
* True for a steering/peer-interrupt placeholder. A synthetic placeholder
* identifies a call that never entered `tool.execute`; an interrupted
* placeholder identifies one that started but threw before returning usable
* output. Both are normal steering control flow and render neutrally (#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";
const details = this.#result.details as
| { __synthetic?: boolean; __interrupted?: boolean; source?: string; execution?: string }
| undefined;
if (details?.source !== "interrupt_skipped") return false;
return details.__synthetic === true || (details.__interrupted === true && details.execution === "started");
}
/**
packages/coding-agent/test/steering-skip-render.test.ts
+13 -11
View File
@@ -23,23 +23,25 @@ function renderSkippedEdit(details: unknown): string {
}
describe("mid-turn steering skip rendering", () => {
it("renders a synthetic interrupt-skip as info, not an error", async () => {
it("renders both pending and in-flight interrupt skips as info, not errors", 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 skipDetails = [
{ __synthetic: true, source: "interrupt_skipped", executed: false },
{ __interrupted: true, source: "interrupt_skipped", execution: "started" },
];
const rendered = renderSkippedEdit({
__synthetic: true,
source: "interrupt_skipped",
executed: false,
});
for (const details of skipDetails) {
const rendered = renderSkippedEdit(details);
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");
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 () => {