fix(session): converge plan mode on ask/resolve across continuation paths

Plan-mode convergence (call `ask` or `resolve`) was enforced on only the
non-synthetic `prompt()` return. Every other turn-ending path — `agent.continue()`
drains, the IRC idle wake, and advisor steer delivery — settles via `agent_end`
and bypassed it, so advisor/IRC/follow-on traffic could keep the agent producing
non-converging turns indefinitely (the prompt() recovery wait never returns while
continuations run, and agent-core re-drains late steering/asides before agent_end).

Suppress non-user producers in plan mode: `#routeAdvice` preserves the advisor
card (visible + persisted, no turn) and the idle IRC paths (`deliverIrcMessage`,
`#resumeStrandedIrcAsides`) record into context without waking a turn. User
steers/follow-ups are untouched and still resume planning.

Enforce the decision at the universal `agent_end` terminal settle via a
bounded-retry counter: a plan-mode turn that stops without `ask`/`resolve` gets
the reminder plus a provider-neutral `required` choice (both tools stay
available), up to a fixed cap, then yields to the user. A non-decision tool
answer (e.g. `read`) does not reset the cap, so it cannot loop or silently end
plan mode un-converged; `ask`/`resolve`, a fresh user prompt, or plan-mode exit
reset the counter. Todo-completion reminders are gated off in plan mode so they
cannot re-wake a turn the cap intends to yield. Removes the now-redundant
post-`prompt()` enforcement to keep a single policy.

Op: correct
Restores: spec:plan-mode turns must converge to ask/resolve or yield after bounded reminders
This commit is contained in:
metaphorics
2026-07-01 00:23:07 +09:00
parent 5bc68f57cd
commit 0d23cb75c2
3 changed files with 374 additions and 17 deletions
+4
View File
@@ -2,6 +2,10 @@
## [Unreleased]
### Fixed
- Fixed plan mode hanging without converging on `ask`/`resolve` after advisor cards, idle IRC messages, or follow-on turns. Plan-mode decision enforcement ran on only the non-synthetic `prompt()` return; continuation/wake paths settled via `agent_end` and bypassed it. Advisor cards and idle IRC are now recorded into context without waking an autonomous turn, and the `ask`/`resolve` decision is enforced at the universal `agent_end` terminal settle via a bounded-retry counter (provider-neutral `required`, both tools kept available) that reminds-then-forces a fixed number of times and then yields to the user — never looping, never silently ending plan mode un-converged. ([#3910](https://github.com/can1357/oh-my-pi/issues/3910))
## [16.2.8] - 2026-06-30
### Added
@@ -340,6 +340,8 @@ import { classifyUnexpectedStop, isUnexpectedStopCandidate } from "./unexpected-
import { YieldQueue } from "./yield-queue";
const SESSION_STOP_CONTINUATION_CAP = 8;
const PLAN_MODE_REMINDER_MAX = 3;
const PLAN_DECISION_TOOLS = new Set(["ask", "resolve"]);
/** Abort reason for the Gemini reasoning-header runaway interrupt. Surfaced on the
* discarded assistant turn only; never reaches the model. */
@@ -1410,6 +1412,8 @@ export class AgentSession {
* instruction") does not drive 1/3 → 2/3 → 3/3 without user input.
*/
#todoReminderAwaitingProgress = false;
#planModeReminderCount = 0;
#planModeReminderAwaitingProgress = false;
#todoPhases: TodoPhase[] = [];
#replanTitleRefreshInFlight: Promise<void> | undefined = undefined;
#toolChoiceQueue = new ToolChoiceQueue();
@@ -1674,6 +1678,21 @@ export class AgentSession {
if (this.#canAutoContinueForFollowUp() && this.agent.hasQueuedMessages()) return;
const records = this.#pendingIrcAsides;
this.#pendingIrcAsides = [];
if (this.#planModeState?.enabled) {
// Plan mode: fold stranded IRC asides into context without waking an
// autonomous turn. Convergence to ask/resolve stays user-driven.
for (const record of records) {
this.agent.appendMessage(record);
this.sessionManager.appendCustomMessageEntry(
record.customType,
record.content,
record.display,
record.details,
record.attribution ?? "agent",
);
}
return;
}
this.#wakeForIrc(records);
}
@@ -2313,6 +2332,20 @@ export class AgentSession {
return;
}
this.#recordAdvisorInterruptDelivered();
if (this.#planModeState?.enabled) {
// Plan mode: record advice visibly in context but never wake an
// autonomous turn — only user-driven turns converge on ask/resolve.
this.#preserveAdvisorCard({
role: "custom",
customType: "advisor",
content,
display: true,
attribution: "agent",
details,
timestamp: Date.now(),
});
return;
}
void this.sendCustomMessage(
{ customType: "advisor", content, display: true, attribution: "agent", details },
{ deliverAs: "steer", triggerTurn: true },
@@ -3132,6 +3165,11 @@ export class AgentSession {
} else {
await this.#goalRuntime.onToolCompleted(event.toolName);
}
this.#planModeReminderAwaitingProgress = false;
if (this.#isPlanDecisionTool(event.toolName)) {
this.#planModeReminderCount = 0;
this.#planModeReminderAwaitingProgress = false;
}
}
if (event.type === "tool_execution_end" && event.toolName === "yield" && !event.isError) {
this.#lastSuccessfulYieldToolCallId = event.toolCallId;
@@ -3549,6 +3587,11 @@ export class AgentSession {
await emitAgentEndNotification();
return;
}
const planModeContinuationScheduled = await this.#enforcePlanModeDecisionAtSettle();
if (planModeContinuationScheduled) {
await emitAgentEndNotification();
return;
}
const todoContinuationScheduled = await this.#checkTodoCompletion();
if (todoContinuationScheduled) {
await emitAgentEndNotification();
@@ -6300,6 +6343,9 @@ export class AgentSession {
if (state?.enabled) {
this.#planReferenceSent = false;
this.#planReferencePath = state.planFilePath;
} else {
this.#planModeReminderCount = 0;
this.#planModeReminderAwaitingProgress = false;
}
}
@@ -6722,6 +6768,8 @@ export class AgentSession {
// Agent-initiated synthetic prompts (auto-continue, plan, reminders) do not.
if (options?.userInitiated ?? !options?.synthetic) {
this.#advisorAutoResumeSuppressed = false;
this.#planModeReminderCount = 0;
this.#planModeReminderAwaitingProgress = false;
}
// If streaming, queue via steer() or followUp() based on option
@@ -6792,9 +6840,6 @@ export class AgentSession {
// (e.g., compaction aborted, validation failed).
this.#toolChoiceQueue.removeByLabel("eager-todo");
}
if (!options?.synthetic) {
await this.#enforcePlanModeToolDecision();
}
return true;
}
@@ -10047,41 +10092,67 @@ export class AgentSession {
this.#checkpointState = undefined;
this.#pendingRewindReport = undefined;
}
async #enforcePlanModeToolDecision(): Promise<void> {
#isPlanDecisionTool(name: string): boolean {
return PLAN_DECISION_TOOLS.has(name);
}
async #enforcePlanModeDecisionAtSettle(): Promise<boolean> {
if (!this.#planModeState?.enabled) {
return;
return false;
}
const assistantMessage = this.#findLastAssistantMessage();
if (!assistantMessage) {
return;
return false;
}
if (assistantMessage.stopReason === "error" || assistantMessage.stopReason === "aborted") {
return;
return false;
}
const calledRequiredTool = assistantMessage.content.some(
content => content.type === "toolCall" && (content.name === "ask" || content.name === "resolve"),
const calledDecisionTool = assistantMessage.content.some(
content => content.type === "toolCall" && this.#isPlanDecisionTool(content.name),
);
if (calledRequiredTool) {
return;
if (calledDecisionTool) {
this.#planModeReminderCount = 0;
this.#planModeReminderAwaitingProgress = false;
return false;
}
const hasToolCall = assistantMessage.content.some(content => content.type === "toolCall");
if (hasToolCall) {
return false;
}
if (this.#planModeReminderAwaitingProgress) {
return false;
}
if (this.#planModeReminderCount >= PLAN_MODE_REMINDER_MAX) {
logger.debug("Plan mode convergence: reminder cap reached; yielding to user");
return false;
}
const hasRequiredTools = this.#toolRegistry.has("ask") && this.#toolRegistry.has("resolve");
if (!hasRequiredTools) {
logger.warn("Plan mode enforcement skipped because ask/resolve tools are unavailable", {
activeToolNames: this.agent.state.tools.map(tool => tool.name),
});
return;
return false;
}
this.#planModeReminderCount++;
this.#planModeReminderAwaitingProgress = true;
this.#toolChoiceQueue.pushOnce("required", { label: "plan-mode-decision" });
const reminder = prompt.render(planModeToolDecisionReminderPrompt, {
askToolName: "ask",
});
const reminderMessage: Message = {
role: "developer",
content: [{ type: "text", text: reminder }],
attribution: "agent",
timestamp: Date.now(),
};
await this.prompt(reminder, {
synthetic: true,
expandPromptTemplates: false,
toolChoice: "required",
});
this.agent.appendMessage(reminderMessage);
this.sessionManager.appendMessage(reminderMessage);
this.#scheduleAgentContinue({ generation: this.#promptGeneration });
return true;
}
/**
@@ -10230,6 +10301,13 @@ export class AgentSession {
return false;
}
// Plan mode owns convergence via #enforcePlanModeDecisionAtSettle (remind →
// cap → yield). Todo reminders must not re-wake a turn the cap intends to
// yield to the user. The label is already consumed above, so no leak.
if (this.#planModeState?.enabled) {
return false;
}
// Suppress within a self-continuation chain: if the agent's last turn was driven by a
// prior reminder (and the agent took no tool-level action since), do not re-ping.
// The agent has already acknowledged; further escalation just wastes context and
@@ -13082,6 +13160,18 @@ export class AgentSession {
if (autoReply) void this.#runIrcAutoReply(msg);
return "injected";
}
// Plan mode: record into context but do not wake an autonomous turn.
if (this.#planModeState?.enabled) {
this.agent.appendMessage(record);
this.sessionManager.appendCustomMessageEntry(
record.customType,
record.content,
record.display,
record.details,
record.attribution ?? "agent",
);
return "injected";
}
// Idle: wake a real turn so the recipient responds (shared with the stranded-aside resume).
this.#wakeForIrc([record]);
return "woken";
@@ -0,0 +1,263 @@
/**
* Contract: plan mode converges on `ask`/`resolve` regardless of how a turn
* ends, and non-user producers cannot keep it spinning.
*
* T1. An advisor concern in plan mode is recorded as a visible card but never
* wakes an autonomous primary turn.
* T2. An idle IRC message in plan mode is folded into context ("injected"),
* not woken.
* T3. A plan-mode turn that stops without `ask`/`resolve` is reminded at the
* terminal settle, bounded by PLAN_MODE_REMINDER_MAX (then yields to the
* user), and either decision tool resets the counter.
*/
import { afterEach, beforeEach, describe, expect, it } from "bun:test";
import { Agent, type AgentMessage, type AgentTool, type StreamFn } from "@oh-my-pi/pi-agent-core";
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";
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
import type { IrcMessage } from "@oh-my-pi/pi-coding-agent/irc/bus";
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
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";
import planModeReminderPrompt from "../src/prompts/system/plan-mode-tool-decision-reminder.md" with { type: "text" };
/** A stable, literal (non-templated) line of the reminder prompt, so the test
* pins the reminder by its real content rather than a hardcoded copy. */
function deriveReminderFragment(template: string): string {
const line = template
.split("\n")
.map(l => l.trim())
.find(l => l.length > 20 && !l.includes("{{"));
if (!line) throw new Error("plan-mode reminder template is missing a stable marker line");
return line;
}
const REMINDER_FRAGMENT = deriveReminderFragment(planModeReminderPrompt);
function makeTool(name: string): AgentTool {
return {
name,
label: name,
description: `Fake ${name}`,
parameters: type({}),
async execute() {
return { content: [{ type: "text" as const, text: "ok" }] };
},
};
}
/** Concatenate the text blocks of a message (string or content-array). */
function messageText(message: AgentMessage): string {
if (!("content" in message)) return "";
const content = message.content;
if (typeof content === "string") return content;
if (!Array.isArray(content)) return "";
return content
.filter(block => block.type === "text")
.map(block => block.text)
.join("\n");
}
function countReminders(messages: readonly AgentMessage[]): number {
return messages.filter(m => m.role === "developer" && messageText(m).includes(REMINDER_FRAGMENT)).length;
}
interface PlanHarness {
session: AgentSession;
mock: MockModel;
advisorMock?: MockModel;
}
describe("AgentSession plan-mode convergence", () => {
let tempDir: TempDir;
let session: AgentSession | undefined;
const authStorages: AuthStorage[] = [];
beforeEach(() => {
tempDir = TempDir.createSync("@pi-plan-converge-");
});
afterEach(async () => {
try {
await session?.dispose();
} finally {
session = undefined;
for (const authStorage of authStorages.splice(0)) authStorage.close();
// dispose() awaits the agent teardown above; no wall-clock wait needed.
await tempDir?.remove();
}
});
async function createPlanSession(
responses: MockResponse[],
options?: { advisorResponses?: MockResponse[] },
): Promise<PlanHarness> {
const model = getBundledModel("anthropic", "claude-sonnet-4-5");
if (!model) throw new Error("Expected bundled anthropic model to exist");
const askTool = makeTool("ask");
const resolveTool = makeTool("resolve");
const readTool = makeTool("read");
const mock = createMockModel({ responses });
const agent = new Agent({
getApiKey: () => "test-key",
// All three tools active so a scripted ask/resolve/read call (and a
// forced "required" choice) can actually execute (isToolChoiceActive).
initialState: { model, systemPrompt: ["Test"], tools: [askTool, resolveTool, readTool], messages: [] },
streamFn: mock.stream,
});
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-${Snowflake.next()}.yml`));
let advisorMock: MockModel | undefined;
let advisorStreamFn: StreamFn | undefined;
if (options?.advisorResponses) {
advisorMock = createMockModel({ responses: options.advisorResponses });
advisorStreamFn = advisorMock.stream;
}
const created = new AgentSession({
agent,
sessionManager: SessionManager.inMemory(),
settings: Settings.isolated({
"compaction.enabled": false,
"retry.enabled": false,
}),
modelRegistry,
toolRegistry: new Map<string, AgentTool>([
["ask", askTool],
["resolve", resolveTool],
["read", readTool],
]),
builtInToolNames: ["ask", "resolve", "read"],
advisorTools: [],
advisorStreamFn,
});
created.setPlanModeState({ enabled: true, planFilePath: "local://PLAN.md" });
session = created;
return { session: created, mock, advisorMock };
}
it("T1: an advisor concern does not wake the primary in plan mode", async () => {
const harness = await createPlanSession([], {
advisorResponses: [
{
content: [
{ type: "toolCall", name: "advise", arguments: { note: "tighten the plan", severity: "concern" } },
],
},
],
});
harness.session.settings.setModelRole("advisor", "anthropic/claude-sonnet-4-5");
expect(harness.session.setAdvisorEnabled(true)).toBe(true);
const advisor = harness.session.getAdvisorAgent();
if (!advisor) throw new Error("Expected advisor agent to be live");
await advisor.prompt("inspect current turn").catch(() => {});
await harness.session.waitForIdle();
const advisorCards = harness.session.agent.state.messages.filter(
m => m.role === "custom" && m.customType === "advisor",
);
expect(advisorCards.length).toBeGreaterThanOrEqual(1);
// The note was preserved/recorded, not used to wake the primary.
expect(harness.mock.calls.length).toBe(0);
// The advisor model actually ran (the test drove the routeAdvice seam).
expect(harness.advisorMock?.calls.length ?? 0).toBeGreaterThanOrEqual(1);
});
it("T2: an idle IRC message does not wake an autonomous turn in plan mode", async () => {
const harness = await createPlanSession([]);
const msg: IrcMessage = { id: "m1", from: "peer", to: "me", body: "ping", ts: Date.now() };
const outcome = await harness.session.deliverIrcMessage(msg);
expect(outcome).toBe("injected");
const sawIrc = harness.session.agent.state.messages.some(
m => m.role === "custom" && m.customType === "irc:incoming",
);
expect(sawIrc).toBe(true);
expect(harness.mock.calls.length).toBe(0);
});
it("T3a: convergence reminders are bounded by the cap, then yield to the user", async () => {
// Alternating text-stop / read cascade: each text stop with awaiting=false
// escalates a reminder; each read clears awaiting (not the count). The 7th
// text stop lands with awaiting=false AND count===cap → cap guard yields.
const harness = await createPlanSession([
{ content: ["planning A"] },
{ content: [{ type: "toolCall", name: "read", arguments: { path: "a" } }] },
{ content: ["planning B"] },
{ content: [{ type: "toolCall", name: "read", arguments: { path: "b" } }] },
{ content: ["planning C"] },
{ content: [{ type: "toolCall", name: "read", arguments: { path: "c" } }] },
{ content: ["planning D"] },
]);
// Todos are ENABLED (default) with an incomplete item, so a missing plan-mode
// gate in #checkTodoCompletion would inject a todo reminder and schedule an
// 8th continuation past the cap — the mock.calls.length === 7 assertion below
// is the behavioral guard for that bypass.
harness.session.setTodoPhases([{ name: "Plan", tasks: [{ content: "draft the plan", status: "pending" }] }]);
await harness.session.prompt("make a plan");
await harness.session.waitForIdle();
// Exactly PLAN_MODE_REMINDER_MAX (3) reminders, then a cap-yield (no 4th,
// no further continuation), and plan mode is still on (not silently exited).
expect(countReminders(harness.session.agent.state.messages)).toBe(3);
expect(harness.mock.calls.length).toBe(7);
expect(harness.session.getPlanModeState()?.enabled).toBe(true);
});
it("T3b: a resolve call resets the convergence counter", async () => {
const harness = await createPlanSession([
{ content: ["planning A"] },
{ content: [{ type: "toolCall", name: "resolve", arguments: { action: "discard", reason: "test reset" } }] },
{ content: ["planning B"] },
{ content: ["planning C"] },
]);
await harness.session.prompt("make a plan");
await harness.session.waitForIdle();
// reminder #1 (text) → resolve resets → reminder #2 (text) → awaiting latch
// suppresses on the final text stop. Two reminders proves the reset re-armed
// the budget; four model calls proves no runaway.
expect(countReminders(harness.session.agent.state.messages)).toBe(2);
expect(harness.mock.calls.length).toBe(4);
});
it("T3c: an ask call resets the convergence counter", async () => {
const harness = await createPlanSession([
{ content: ["planning A"] },
{
content: [
{
type: "toolCall",
name: "ask",
arguments: {
questions: [
{ id: "q", question: "which?", options: [{ label: "a" }, { label: "b" }], recommended: 0 },
],
},
},
],
},
{ content: ["planning B"] },
{ content: ["planning C"] },
]);
await harness.session.prompt("make a plan");
await harness.session.waitForIdle();
expect(countReminders(harness.session.agent.state.messages)).toBe(2);
expect(harness.mock.calls.length).toBe(4);
});
});