fix(agent): keep queued non-interruptible tools alive on peer-IRC interrupt
A peer-IRC interrupt (e.g. a subagent message) aborts only interruptible waits and leaves already-running non-interruptible foreground work alone. But runTool's `interruptState.triggered` early-return skipped every not-yet-started tool regardless of source, so a non-interruptible tool queued behind an interruptible wait in the same batch (a batched todo/write after `hub wait`) was dropped with "Skipped due to pending peer interrupt". Exclude non-interruptible tools from the early skip on the IRC path; user and system steering still preempt all queued work. Fixes #7493
This commit is contained in:
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- A peer-IRC interrupt (e.g. a subagent message) no longer skips a non-interruptible tool call that was queued behind an interruptible wait in the same batch, so a batched `todo`/`write` update ordered after `hub wait` now runs instead of failing with "Skipped due to pending peer interrupt"; IRC still aborts only interruptible waits, and user/system steering still preempts queued work ([#7493](https://github.com/can1357/oh-my-pi/issues/7493)).
|
||||
|
||||
## [17.2.5] - 2026-08-03
|
||||
|
||||
### Breaking Changes
|
||||
|
||||
@@ -2393,7 +2393,16 @@ async function executeToolCalls(
|
||||
};
|
||||
|
||||
const runTool = async (record: (typeof records)[number], index: number): Promise<void> => {
|
||||
if (interruptState.triggered) {
|
||||
// A pending interrupt preempts not-yet-started tools so the message
|
||||
// injects promptly. A peer-IRC interrupt is the exception: it aborts
|
||||
// interruptible waits only and leaves non-interruptible foreground work
|
||||
// untouched (see the emit branch below and the `does not abort a
|
||||
// non-interruptible foreground tool` case). That guarantee must hold for
|
||||
// work still queued behind the aborted wait too — otherwise a batched
|
||||
// `todo`/`write` gets dropped as "Skipped due to pending peer interrupt"
|
||||
// purely for being ordered after the wait (#7493). User/system steering
|
||||
// still preempts everything queued.
|
||||
if (interruptState.triggered && (record.interruptible || interruptState.source !== "irc")) {
|
||||
// Skip both span emission and the collector orphan record here. The
|
||||
// tail sweep below (after `Promise.allSettled`) is the single path
|
||||
// that handles "no result message was produced" — it calls
|
||||
|
||||
@@ -2121,6 +2121,95 @@ describe("agentLoop with AgentMessage", () => {
|
||||
}
|
||||
});
|
||||
|
||||
it("runs a queued non-interruptible tool after an IRC interrupt aborts an earlier wait (#7493)", async () => {
|
||||
// Reproduces the reporter's orchestration flow: a batch pairs an
|
||||
// interruptible `hub wait` with a non-interruptible `todo` update queued
|
||||
// behind it (todo is `concurrency: "exclusive"`). A peer subagent message
|
||||
// (IRC) lands mid-wait, aborting the wait. The queued todo had not started
|
||||
// yet, so the `interruptState.triggered` early-return in `runTool` skipped
|
||||
// it — surfacing as "Skipped due to pending peer interrupt". IRC must leave
|
||||
// non-interruptible foreground work alone whether it is already running or
|
||||
// still queued, so the todo update must actually execute.
|
||||
const toolSchema = type({});
|
||||
let ircReady = false;
|
||||
let ircDrained = false;
|
||||
let todoExecuted = false;
|
||||
const ircMessage = createUserMessage("peer irc");
|
||||
|
||||
const wait: AgentTool<typeof toolSchema, Record<string, never>> = {
|
||||
name: "wait",
|
||||
label: "Wait",
|
||||
description: "Interruptible wait (mimics a job poll)",
|
||||
parameters: toolSchema,
|
||||
interruptible: true,
|
||||
async execute(_toolCallId, _params, signal) {
|
||||
ircReady = true;
|
||||
// Resolve strictly on the IRC abort under test — no wall-clock timer.
|
||||
// If the interrupt never fired the loop would hang, which is itself
|
||||
// the failure signal (ts-no-test-timers: await the real event).
|
||||
const { promise, resolve } = Promise.withResolvers<void>();
|
||||
if (signal?.aborted) resolve();
|
||||
else signal?.addEventListener("abort", () => resolve(), { once: true });
|
||||
await promise;
|
||||
return { content: [{ type: "text", text: "waited" }], details: {} };
|
||||
},
|
||||
};
|
||||
|
||||
const todo: AgentTool<typeof toolSchema, Record<string, never>> = {
|
||||
name: "todo",
|
||||
label: "Todo",
|
||||
description: "Non-interruptible local state mutation (mimics todo)",
|
||||
parameters: toolSchema,
|
||||
concurrency: "exclusive",
|
||||
async execute() {
|
||||
todoExecuted = true;
|
||||
return { content: [{ type: "text", text: "todo updated" }], details: {} };
|
||||
},
|
||||
};
|
||||
|
||||
const context: AgentContext = { systemPrompt: [""], messages: [], tools: [wait, todo] };
|
||||
const mock = createMockModel({
|
||||
responses: [
|
||||
{
|
||||
content: [
|
||||
{ type: "toolCall", id: "call-wait", name: "wait", arguments: {} },
|
||||
{ type: "toolCall", id: "call-todo", name: "todo", arguments: {} },
|
||||
],
|
||||
},
|
||||
{ content: ["done"] },
|
||||
],
|
||||
});
|
||||
const config: AgentLoopConfig = {
|
||||
model: mock.model,
|
||||
convertToLlm: identityConverter,
|
||||
interruptMode: "immediate",
|
||||
hasIrcInterrupts: () => ircReady && !ircDrained,
|
||||
getAsideMessages: async () => {
|
||||
if (ircReady && !ircDrained) {
|
||||
ircDrained = true;
|
||||
return [() => ircMessage];
|
||||
}
|
||||
return [];
|
||||
},
|
||||
};
|
||||
|
||||
const events: AgentEvent[] = [];
|
||||
for await (const event of agentLoop([createUserMessage("start")], context, config, undefined, mock.stream)) {
|
||||
events.push(event);
|
||||
}
|
||||
|
||||
expect(ircDrained).toBe(true);
|
||||
expect(todoExecuted).toBe(true);
|
||||
const todoEnd = events.find(
|
||||
(e): e is Extract<AgentEvent, { type: "tool_execution_end" }> =>
|
||||
e.type === "tool_execution_end" && e.toolCallId === "call-todo",
|
||||
);
|
||||
expect(todoEnd?.isError).toBe(false);
|
||||
if (todoEnd?.result.content[0]?.type === "text") {
|
||||
expect(todoEnd.result.content[0].text).toContain("todo updated");
|
||||
}
|
||||
});
|
||||
|
||||
it("does not abort a tool when its interruptibility resolver rejects the call", async () => {
|
||||
const toolSchema = type({ op: "'start' | 'wait'" });
|
||||
let steerReady = false;
|
||||
|
||||
Reference in New Issue
Block a user