Merge PR #4971: fix(agent): stop terminal yield loops after IRC wake
This commit is contained in:
@@ -5,6 +5,7 @@
|
||||
### Fixed
|
||||
|
||||
- Fixed remote compaction for Codex Responses Lite models (GPT-5.6 family): both the V1 `/responses/compact` request and the V2 `compaction_trigger` stream now apply the lite rewrite (instructions as an input item, no top-level `instructions`/`tools`, `all_turns` reasoning replay on V2) and send the `x-openai-internal-codex-responses-lite` header, matching codex-rs routing compaction through `build_responses_request`.
|
||||
- Fixed aborted tool-result hooks from continuing into another provider call before the abort settled. ([#4963](https://github.com/can1357/oh-my-pi/issues/4963))
|
||||
|
||||
## [16.3.12] - 2026-07-08
|
||||
|
||||
|
||||
@@ -1084,6 +1084,12 @@ async function runLoopBody(
|
||||
}
|
||||
}
|
||||
|
||||
// A tool hook may abort to mark the tool result as terminal (e.g. subagent yield).
|
||||
// Stop before the next provider call; event listeners observe the result too late.
|
||||
if (signal?.aborted) {
|
||||
hasMoreToolCalls = false;
|
||||
}
|
||||
|
||||
if (toolCalls.length > 0) {
|
||||
pausedTurnContinuations = 0;
|
||||
} else if (
|
||||
|
||||
@@ -16,6 +16,7 @@
|
||||
- Fixed subagent `yield` tool calls being discarded when the soft request budget hard-aborted the same assistant turn before the yield result event landed. ([#5006](https://github.com/can1357/oh-my-pi/issues/5006))
|
||||
- Fixed `--tools` filtering in interactive sessions disabling deferred MCP tools; MCP tools discovered from configured servers now stay active when the flag limits only built-in tools. ([#5013](https://github.com/can1357/oh-my-pi/issues/5013))
|
||||
|
||||
- Fixed kept-alive task subagents entering a repeated provider-call loop after an IRC wake and terminal `yield`. ([#4963](https://github.com/can1357/oh-my-pi/issues/4963))
|
||||
## [16.3.15] - 2026-07-09
|
||||
|
||||
### Changed
|
||||
|
||||
@@ -1868,6 +1868,7 @@ export class AgentSession {
|
||||
* Cleared before every new prompt turn so the next turn evaluates cleanly.
|
||||
*/
|
||||
#yieldTerminationPending = false;
|
||||
#synchronouslyTerminatedYieldToolCallIds = new Set<string>();
|
||||
#providerSessionState = new Map<string, ProviderSessionState>();
|
||||
#hindsightSessionState: HindsightSessionState | undefined = undefined;
|
||||
readonly rawSseDebugBuffer: RawSseDebugBuffer;
|
||||
@@ -2246,8 +2247,8 @@ export class AgentSession {
|
||||
this.#maybeAbortStreamingEdit(event);
|
||||
this.#maybeInterruptGeminiHeaderRunaway(message, assistantMessageEvent);
|
||||
});
|
||||
// Per-tool TTSR reminders are folded into the matched tool's result via this hook.
|
||||
this.agent.afterToolCall = ctx => this.#ttsrAfterToolCall(ctx);
|
||||
// Tool-result hook owns synchronous post-tool actions that must affect the current loop.
|
||||
this.agent.afterToolCall = ctx => this.#afterToolCall(ctx);
|
||||
this.agent.providerSessionState = this.#providerSessionState;
|
||||
this.#syncAgentSessionId();
|
||||
this.#syncTodoPhasesFromBranch();
|
||||
@@ -3712,9 +3713,12 @@ export class AgentSession {
|
||||
this.#planModeReminderAwaitingProgress = false;
|
||||
}
|
||||
}
|
||||
if (event.type === "tool_execution_end" && event.toolName === "yield" && !event.isError) {
|
||||
this.#lastSuccessfulYieldToolCallId = event.toolCallId;
|
||||
this.#yieldTerminationPending = true;
|
||||
if (event.type === "tool_execution_end" && this.#isTerminalYieldToolResult(event)) {
|
||||
const alreadyTerminated = this.#synchronouslyTerminatedYieldToolCallIds.delete(event.toolCallId);
|
||||
if (!alreadyTerminated) {
|
||||
this.#markTerminalYieldToolCall(event.toolCallId);
|
||||
this.agent.abort();
|
||||
}
|
||||
}
|
||||
|
||||
// TTSR: Check for pattern matches on assistant text/thinking and tool argument deltas
|
||||
@@ -4459,6 +4463,21 @@ export class AgentSession {
|
||||
}
|
||||
}
|
||||
|
||||
#afterToolCall(ctx: AfterToolCallContext): AfterToolCallResult | undefined {
|
||||
if (
|
||||
this.#isTerminalYieldToolResult({
|
||||
toolName: ctx.toolCall.name,
|
||||
isError: ctx.isError,
|
||||
result: ctx.result,
|
||||
})
|
||||
) {
|
||||
this.#markTerminalYieldToolCall(ctx.toolCall.id);
|
||||
this.#synchronouslyTerminatedYieldToolCallIds.add(ctx.toolCall.id);
|
||||
this.agent.abort();
|
||||
}
|
||||
return this.#ttsrAfterToolCall(ctx);
|
||||
}
|
||||
|
||||
/** `afterToolCall` hook: fold any per-tool TTSR reminders into the result. */
|
||||
#ttsrAfterToolCall(ctx: AfterToolCallContext): AfterToolCallResult | undefined {
|
||||
const rules = this.#perToolTtsrInjections.get(ctx.toolCall.id);
|
||||
@@ -10677,6 +10696,24 @@ export class AgentSession {
|
||||
}
|
||||
return COMPACTION_CHECK_NONE;
|
||||
}
|
||||
#isTerminalYieldToolResult(event: { toolName: string; isError?: boolean; result?: { details?: unknown } }): boolean {
|
||||
if (event.toolName !== "yield" || event.isError) return false;
|
||||
const details = event.result?.details;
|
||||
if (!details || typeof details !== "object") return true;
|
||||
const record = details as Record<string, unknown>;
|
||||
return !(
|
||||
record.status === "success" &&
|
||||
Array.isArray(record.type) &&
|
||||
record.type.length > 0 &&
|
||||
record.type.every(item => typeof item === "string")
|
||||
);
|
||||
}
|
||||
|
||||
#markTerminalYieldToolCall(toolCallId: string): void {
|
||||
this.#lastSuccessfulYieldToolCallId = toolCallId;
|
||||
this.#yieldTerminationPending = true;
|
||||
}
|
||||
|
||||
#assistantMessageHasSuccessfulYieldToolCall(assistantMessage: AssistantMessage, toolCallId: string): boolean {
|
||||
const lastToolCall = assistantMessage.content
|
||||
.slice()
|
||||
|
||||
@@ -18,7 +18,8 @@
|
||||
* follow-up stays queued for the next explicit resume rather than auto-running.
|
||||
*/
|
||||
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
|
||||
import { Agent, type AgentMessage } from "@oh-my-pi/pi-agent-core";
|
||||
import { Agent, type AgentMessage, type AgentTool } from "@oh-my-pi/pi-agent-core";
|
||||
import type { ToolCall } from "@oh-my-pi/pi-ai";
|
||||
import { createMockModel, type MockModel, type MockResponse } from "@oh-my-pi/pi-ai/providers/mock";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
@@ -29,6 +30,18 @@ import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { USER_INTERRUPT_LABEL } from "@oh-my-pi/pi-coding-agent/session/messages";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { Snowflake, TempDir } from "@oh-my-pi/pi-utils";
|
||||
import { type } from "arktype";
|
||||
|
||||
interface MockYieldDetails {
|
||||
status: "success";
|
||||
data?: unknown;
|
||||
type?: string | string[];
|
||||
}
|
||||
|
||||
const mockYieldParameters = type({
|
||||
result: "unknown",
|
||||
"type?": "unknown",
|
||||
});
|
||||
|
||||
const ADVISOR_TYPE = "advisor";
|
||||
|
||||
@@ -94,6 +107,56 @@ describe("AgentSession advisor auto-resume suppression", () => {
|
||||
return { session, sessionManager, mock, streamStarted: started.promise };
|
||||
}
|
||||
|
||||
function readYieldResultData(result: unknown): unknown {
|
||||
if (!result || typeof result !== "object" || !("data" in result)) return undefined;
|
||||
return result.data;
|
||||
}
|
||||
|
||||
function isYieldType(value: unknown): value is string | string[] {
|
||||
return (
|
||||
typeof value === "string" ||
|
||||
(Array.isArray(value) && value.length > 0 && value.every(item => typeof item === "string"))
|
||||
);
|
||||
}
|
||||
|
||||
function createMockYieldTool(): AgentTool<typeof mockYieldParameters, MockYieldDetails> {
|
||||
return {
|
||||
name: "yield",
|
||||
label: "Yield",
|
||||
description: "Mock yield tool",
|
||||
parameters: mockYieldParameters,
|
||||
execute: async (_toolCallId, params) => {
|
||||
const details: MockYieldDetails = { status: "success", data: readYieldResultData(params.result) };
|
||||
if (isYieldType(params.type)) details.type = params.type;
|
||||
return {
|
||||
content: [{ type: "text", text: "Result submitted." }],
|
||||
details,
|
||||
};
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function createYieldMockResponse(args: { result: { data: unknown }; type?: string | string[] }): MockResponse {
|
||||
const toolCall: ToolCall = {
|
||||
type: "toolCall",
|
||||
id: `call_yield_${Snowflake.next()}`,
|
||||
name: "yield",
|
||||
arguments: args,
|
||||
};
|
||||
return {
|
||||
content: [toolCall],
|
||||
stopReason: "toolUse",
|
||||
usage: {
|
||||
input: 1,
|
||||
output: 1,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 2,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
};
|
||||
}
|
||||
|
||||
function advisorCard(content: string) {
|
||||
return {
|
||||
customType: ADVISOR_TYPE,
|
||||
@@ -325,6 +388,41 @@ describe("AgentSession advisor auto-resume suppression", () => {
|
||||
expect(mock.calls.length).toBe(2);
|
||||
});
|
||||
|
||||
it("stops an idle IRC wake after a terminal yield", async () => {
|
||||
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
|
||||
if (!model) throw new Error("Expected claude-sonnet-4-5 model to exist");
|
||||
let providerCalls = 0;
|
||||
const mock = createMockModel({
|
||||
handler: () => {
|
||||
providerCalls++;
|
||||
if (providerCalls > 1) {
|
||||
throw new Error("terminal yield must not start a second provider call");
|
||||
}
|
||||
return createYieldMockResponse({ result: { data: { ok: true } } });
|
||||
},
|
||||
});
|
||||
const agent = new Agent({
|
||||
getApiKey: () => "test-key",
|
||||
initialState: { model, systemPrompt: ["Test"], tools: [createMockYieldTool()] },
|
||||
streamFn: mock.stream,
|
||||
});
|
||||
const sessionManager = SessionManager.inMemory();
|
||||
const settings = Settings.isolated({ "compaction.enabled": false });
|
||||
const authStorage = await AuthStorage.create(tempDir.join(`auth-${Snowflake.next()}.db`));
|
||||
authStorages.push(authStorage);
|
||||
authStorage.setRuntimeApiKey("anthropic", "test-key");
|
||||
const modelRegistry = new ModelRegistry(authStorage, tempDir.join("models.yml"));
|
||||
session = new AgentSession({ agent, sessionManager, settings, modelRegistry });
|
||||
const msg: IrcMessage = { id: "m-yield", from: "peer", to: "me", body: "status?", ts: Date.now() };
|
||||
|
||||
const outcome = await session.deliverIrcMessage(msg);
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(outcome).toBe("woken");
|
||||
expect(providerCalls).toBe(1);
|
||||
expect(mock.calls.length).toBe(1);
|
||||
});
|
||||
|
||||
it("flushes an accepted IRC aside on dispose instead of dropping it", async () => {
|
||||
const { session, streamStarted } = await createParkedSession();
|
||||
const running = session.prompt("do the thing");
|
||||
|
||||
@@ -1,11 +1,11 @@
|
||||
/**
|
||||
* Regression: a trailing empty assistant `stop` arriving after a successful
|
||||
* `yield` must NOT trigger empty-stop retry or any other auto-continuation.
|
||||
* Regression: a terminal `yield` must stop the current prompt loop before a
|
||||
* provider continuation can produce a trailing empty assistant `stop`.
|
||||
*
|
||||
* The session's executor treats a successful yield as the terminal result for
|
||||
* a scripted subagent run; if the empty-stop recovery path then schedules
|
||||
* `agent.continue()`, the already-yielded child resumes and produces post-yield
|
||||
* tool calls (see issue #3389).
|
||||
* a scripted subagent run; if the loop continues after that tool result, the
|
||||
* already-yielded child resumes and can enter post-yield retries or tool calls
|
||||
* (see issues #3389 and #4963).
|
||||
*/
|
||||
import { afterEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
@@ -147,20 +147,17 @@ afterEach(async () => {
|
||||
});
|
||||
|
||||
describe("AgentSession yield empty-stop suppression", () => {
|
||||
it("does not retry a trailing empty assistant stop after a successful yield", async () => {
|
||||
const { session, mock } = await createHarness([yieldCall("done", "call-yield-done"), emptyStop()]);
|
||||
it("does not continue to a trailing empty assistant stop after a successful yield", async () => {
|
||||
const { session, mock } = await createHarness([yieldCall("done", "call-yield-done")]);
|
||||
|
||||
await session.prompt("do work then yield");
|
||||
await session.waitForIdle();
|
||||
|
||||
// Two model calls: the yield turn and the trailing empty stop. Without the
|
||||
// fix, the empty stop would schedule a `continue()` and either drive a
|
||||
// third call or throw "no response configured" from the mock.
|
||||
expect(mock.calls).toHaveLength(2);
|
||||
expect(mock.calls).toHaveLength(1);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("suppresses multiple trailing empty stops within the same yield-terminated run", async () => {
|
||||
it("stops at the terminal yield instead of consuming scripted trailing empty stops", async () => {
|
||||
const { session, mock } = await createHarness([
|
||||
yieldCall("done", "call-yield-multi"),
|
||||
emptyStop(),
|
||||
@@ -171,18 +168,14 @@ describe("AgentSession yield empty-stop suppression", () => {
|
||||
await session.prompt("yield then maybe trail");
|
||||
await session.waitForIdle();
|
||||
|
||||
// Without suppression, empty-stop retries would consume extra mock entries
|
||||
// and append at least one reminder. With the fix, the loop ends at the
|
||||
// first trailing empty stop.
|
||||
expect(mock.calls).toHaveLength(2);
|
||||
expect(mock.calls).toHaveLength(1);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(0);
|
||||
});
|
||||
|
||||
it("clears yield-termination on the next prompt so empty stops retry normally", async () => {
|
||||
const { session, mock } = await createHarness([
|
||||
// Run 1: yield then trailing empty stop. Suppression applies.
|
||||
// Run 1: terminal yield stops without consuming a trailing provider response.
|
||||
yieldCall("first", "call-yield-first"),
|
||||
emptyStop(),
|
||||
// Run 2: empty stop should retry as usual now that the flag has cleared.
|
||||
recordCall("alpha", "call-record-alpha"),
|
||||
emptyStop(),
|
||||
@@ -191,7 +184,7 @@ describe("AgentSession yield empty-stop suppression", () => {
|
||||
|
||||
await session.prompt("yield first");
|
||||
await session.waitForIdle();
|
||||
expect(mock.calls).toHaveLength(2);
|
||||
expect(mock.calls).toHaveLength(1);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(0);
|
||||
|
||||
await session.prompt("now record");
|
||||
@@ -199,15 +192,14 @@ describe("AgentSession yield empty-stop suppression", () => {
|
||||
|
||||
// Three additional calls (record, emptyStop, finished). Exactly one
|
||||
// empty-stop reminder injected on the second run.
|
||||
expect(mock.calls).toHaveLength(5);
|
||||
expect(mock.calls).toHaveLength(4);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(1);
|
||||
});
|
||||
|
||||
it("treats an idle IRC wake after a yielded run as a fresh turn for empty-stop retry", async () => {
|
||||
const { session, mock } = await createHarness([
|
||||
// Run 1: yield then trailing empty stop. Suppression applies only to this yielded run.
|
||||
// Run 1: terminal yield stops without consuming a trailing provider response.
|
||||
yieldCall("first", "call-yield-before-irc"),
|
||||
emptyStop(),
|
||||
// Run 2: an idle IRC wake is a fresh turn, so its empty stop should retry normally.
|
||||
emptyStop(),
|
||||
{ content: ["recovered after IRC retry"], stopReason: "stop" },
|
||||
@@ -215,7 +207,7 @@ describe("AgentSession yield empty-stop suppression", () => {
|
||||
|
||||
await session.prompt("yield first");
|
||||
await session.waitForIdle();
|
||||
expect(mock.calls).toHaveLength(2);
|
||||
expect(mock.calls).toHaveLength(1);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(0);
|
||||
|
||||
const outcome = await session.deliverIrcMessage({
|
||||
@@ -228,7 +220,7 @@ describe("AgentSession yield empty-stop suppression", () => {
|
||||
expect(outcome).toBe("woken");
|
||||
await session.waitForIdle();
|
||||
|
||||
expect(mock.calls).toHaveLength(4);
|
||||
expect(mock.calls).toHaveLength(3);
|
||||
expect(reminderMessages(session.agent.state.messages)).toHaveLength(1);
|
||||
expect(assistantText(session.agent.state.messages)).toContain("recovered after IRC retry");
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user