fix(coding-agent): stopped print mode hanging on plan.defaultOnStartup
Headless `omp -p` armed an interactive plan-review flow whenever plan.defaultOnStartup was set. Its only headless exit was a watcher that fired on a successful `xd://propose` execute-dispatch, so any turn where the model did not emit exactly that dispatch stranded until --max-time, printing nothing. --plan-yolo could not rescue it because print mode's arming clashed with the prewalk coordinator's plan-yolo handoff. Print mode no longer honors the startup default: headless has no surface to review, approve, or exit a plan. It writes a one-line stderr note and runs the prompt normally. --plan-yolo remains the deterministic headless plan flow and no longer clashes. Fixes #8272
This commit is contained in:
roboomp
@@ -2,6 +2,10 @@
|
||||
|
||||
## [Unreleased]
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `plan.defaultOnStartup: true` making headless `omp -p` runs hang until `--max-time` with no output. Print mode armed an interactive plan-review flow whose only headless exit was the model emitting a valid `xd://propose` execute-dispatch, so any turn that did not stranded to the deadline. Print mode no longer honors the startup default (it has no surface to review, approve, or exit a plan); `--plan-yolo` remains the supported headless plan flow ([#8272](https://github.com/can1357/oh-my-pi/issues/8272)).
|
||||
|
||||
## [17.2.14] - 2026-08-11
|
||||
|
||||
### Added
|
||||
|
||||
@@ -8,11 +8,9 @@
|
||||
import type { AgentMessage } from "@oh-my-pi/pi-agent-core";
|
||||
import type { ImageContent } from "@oh-my-pi/pi-ai";
|
||||
import { logger, sanitizeText } from "@oh-my-pi/pi-utils";
|
||||
import { resolvePlanModelTransition } from "../plan-mode/model-transition";
|
||||
import { type AgentSession, type AgentSessionEvent, SHUTDOWN_CONSOLIDATE_BUDGET_MS } from "../session/agent-session";
|
||||
import { isSilentAbort } from "../session/messages";
|
||||
import { flushTelemetryExport } from "../telemetry-export";
|
||||
import { PROPOSE_DEVICE_NAME, writeDeviceDispatch } from "../tools/resolve";
|
||||
import { initializeExtensions } from "./runtime-init";
|
||||
|
||||
/**
|
||||
@@ -128,66 +126,27 @@ export async function runPrintMode(session: AgentSession, options: PrintModeOpti
|
||||
},
|
||||
});
|
||||
|
||||
// InteractiveMode applies the same startup default during TUI initialization.
|
||||
// Print mode has no TUI bootstrap, so arm the shared session directly before
|
||||
// the first prompt; persisting the mode_change also lets a later interactive
|
||||
// attachment restore and review the generated plan.
|
||||
let abortAfterPlanProposal = false;
|
||||
const planDefaultArmed =
|
||||
// `plan.defaultOnStartup` opens fresh *interactive* sessions in plan mode so a
|
||||
// human can review the plan before it executes. Headless print mode has no
|
||||
// surface to review, approve, or exit a plan from, and the turn carries no
|
||||
// deterministic way out of plan mode — the model must voluntarily emit a valid
|
||||
// `xd://propose` execute-dispatch, and when it does not the run strands until
|
||||
// the deadline (issue #8272). So do not honor the startup default here; the
|
||||
// supported headless plan flow is `--plan-yolo` (auto-approve → implement),
|
||||
// which is wired independently through the prewalk coordinator.
|
||||
const planStartupIgnored =
|
||||
session.settings.get("plan.defaultOnStartup") &&
|
||||
session.settings.get("plan.enabled") &&
|
||||
session.sessionManager.buildSessionContext().messages.length === 0 &&
|
||||
!session.sessionManager.getEntries().some(entry => entry.type === "mode_change");
|
||||
if (planDefaultArmed) {
|
||||
const planFilePath = session.getPlanReferencePath() || "local://PLAN.md";
|
||||
const previousTools = session.getEnabledToolNames();
|
||||
const planTools = session.hasBuiltInTool("write") ? [...new Set([...previousTools, "write"])] : previousTools;
|
||||
await session.setActiveToolsByName(planTools);
|
||||
session.setPlanModeState({
|
||||
enabled: true,
|
||||
planFilePath,
|
||||
workflow: "parallel",
|
||||
});
|
||||
session.sessionManager.appendModeChange("plan", { planFilePath });
|
||||
abortAfterPlanProposal = true;
|
||||
session.setPlanProposalHandler(async title => {
|
||||
const result = await session.preparePlanForReview(title);
|
||||
const details = result.details;
|
||||
if (details) {
|
||||
const state = session.getPlanModeState();
|
||||
if (state?.enabled) {
|
||||
session.setPlanModeState({ ...state, planFilePath: details.planFilePath });
|
||||
}
|
||||
session.sessionManager.appendModeChange("plan", { planFilePath: details.planFilePath });
|
||||
}
|
||||
return result;
|
||||
});
|
||||
|
||||
const resolved = session.resolveRoleModelWithThinking("plan");
|
||||
const transition = resolvePlanModelTransition(session.model, resolved, false);
|
||||
if (transition.kind === "thinking") {
|
||||
session.setThinkingLevel(transition.thinkingLevel);
|
||||
} else if (transition.kind === "apply") {
|
||||
try {
|
||||
await session.setModelTemporary(transition.model, transition.thinkingLevel);
|
||||
} catch (error) {
|
||||
logger.warn("Failed to switch to plan model for print mode", { error: String(error) });
|
||||
}
|
||||
}
|
||||
if (planStartupIgnored) {
|
||||
process.stderr.write(
|
||||
"Note: plan.defaultOnStartup is ignored in print mode (no interactive surface to review the plan). Use --plan-yolo for a headless plan flow.\n",
|
||||
);
|
||||
}
|
||||
|
||||
// Always subscribe to enable session persistence via _handleAgentEvent
|
||||
session.subscribe(event => {
|
||||
if (abortAfterPlanProposal && event.type === "tool_execution_end" && !event.isError) {
|
||||
const dispatch = writeDeviceDispatch(event.toolName, event.result);
|
||||
if (dispatch?.tool === PROPOSE_DEVICE_NAME && dispatch.mode === "execute") {
|
||||
abortAfterPlanProposal = false;
|
||||
session.markPlanInternalAbortPending();
|
||||
void session.abort().finally(() => {
|
||||
session.clearPlanInternalAbortPending();
|
||||
});
|
||||
}
|
||||
}
|
||||
// In JSON mode, output all events
|
||||
if (mode === "json") {
|
||||
writeStdoutLine(`${JSON.stringify(printableEvent(event))}\n`);
|
||||
|
||||
@@ -0,0 +1,110 @@
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as fs from "node:fs";
|
||||
import * as os from "node:os";
|
||||
import * as path from "node:path";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import type { Context } from "@oh-my-pi/pi-ai";
|
||||
import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { runPrintMode } from "@oh-my-pi/pi-coding-agent/modes/print-mode";
|
||||
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 { createTools, type ToolSession } from "@oh-my-pi/pi-coding-agent/tools";
|
||||
import { Snowflake } from "@oh-my-pi/pi-utils";
|
||||
|
||||
// Regression for #8272: with plan.defaultOnStartup:true, a headless `omp -p`
|
||||
// used to arm plan mode before the initial prompt. The only headless plan-exit
|
||||
// was a watcher that fires on a successful `xd://propose` execute-dispatch, so a
|
||||
// model that never emits exactly that dispatch (the natural plan-mode behavior:
|
||||
// keep trying to finalize the plan) left the turn hanging until the deadline.
|
||||
//
|
||||
// The mock below mirrors that: while plan mode is enabled it keeps attempting to
|
||||
// finalize a plan (write xd://propose) without a plan artifact, which errors and
|
||||
// never produces the propose/execute dispatch. Out of plan mode it answers.
|
||||
describe("print mode + plan.defaultOnStartup (#8272)", () => {
|
||||
let tempDir: string;
|
||||
let authStorage: AuthStorage;
|
||||
let session: AgentSession;
|
||||
let stdoutOutput: string[];
|
||||
|
||||
const holder: { session?: AgentSession } = {};
|
||||
|
||||
beforeEach(async () => {
|
||||
tempDir = path.join(os.tmpdir(), `omp-8272-${Snowflake.next()}`);
|
||||
fs.mkdirSync(tempDir, { recursive: true });
|
||||
stdoutOutput = [];
|
||||
vi.spyOn(process.stdout, "write").mockImplementation((...args: unknown[]) => {
|
||||
const chunk = args[0];
|
||||
if (typeof chunk === "string") stdoutOutput.push(chunk);
|
||||
const last = args[args.length - 1];
|
||||
if (typeof last === "function") (last as () => void)();
|
||||
return true;
|
||||
});
|
||||
vi.spyOn(process.stderr, "write").mockImplementation(() => true);
|
||||
|
||||
const settingsOverrides = { "plan.defaultOnStartup": true, "plan.enabled": true } as const;
|
||||
const toolSession: ToolSession = {
|
||||
cwd: tempDir,
|
||||
hasUI: false,
|
||||
getSessionFile: () => null,
|
||||
getSessionSpawns: () => "*",
|
||||
settings: Settings.isolated(settingsOverrides),
|
||||
};
|
||||
const tools = await createTools(toolSession);
|
||||
|
||||
const model = createMockModel({
|
||||
id: "mock-plan",
|
||||
handler: (_context: Context) => {
|
||||
// Reflect the real model's plan-mode behavior: driven by the plan
|
||||
// prompt, it keeps trying to finalize a plan. Headless there is no
|
||||
// artifact and no surface to fix one, so this never succeeds.
|
||||
if (holder.session?.getPlanModeState?.()?.enabled) {
|
||||
return {
|
||||
content: [
|
||||
{ type: "toolCall", name: "write", arguments: { path: "xd://propose", content: "the-plan" } },
|
||||
],
|
||||
delayMs: 5,
|
||||
};
|
||||
}
|
||||
return { content: ["OK"] };
|
||||
},
|
||||
});
|
||||
const agent = new Agent({
|
||||
getApiKey: () => "mock-key",
|
||||
initialState: { model, systemPrompt: ["Test"], tools },
|
||||
streamFn: (m, context, options) => model.stream(m, context, options),
|
||||
});
|
||||
|
||||
authStorage = await AuthStorage.create(path.join(tempDir, "auth.db"));
|
||||
authStorage.setRuntimeApiKey("mock", "mock-key");
|
||||
const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "models.yml"));
|
||||
session = new AgentSession({
|
||||
agent,
|
||||
sessionManager: SessionManager.inMemory(),
|
||||
settings: Settings.isolated(settingsOverrides),
|
||||
modelRegistry,
|
||||
});
|
||||
holder.session = session;
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
await session.abort().catch(() => {});
|
||||
await session.dispose().catch(() => {});
|
||||
authStorage.close();
|
||||
if (fs.existsSync(tempDir)) fs.rmSync(tempDir, { recursive: true, force: true });
|
||||
vi.restoreAllMocks();
|
||||
holder.session = undefined;
|
||||
});
|
||||
|
||||
it("completes the turn and prints output instead of hanging to the deadline", async () => {
|
||||
// If the startup default re-armed plan mode, the mock would loop on
|
||||
// `xd://propose` forever and this await would never resolve — the runner's
|
||||
// own per-test timeout then fails it, exactly the #8272 symptom.
|
||||
await runPrintMode(session, { mode: "text", initialMessage: "Reply with exactly: OK" });
|
||||
|
||||
expect(stdoutOutput.join("")).toContain("OK");
|
||||
expect(session.getPlanModeState()).toBeUndefined();
|
||||
});
|
||||
});
|
||||
@@ -7,7 +7,7 @@ import {
|
||||
} from "@oh-my-pi/pi-coding-agent/modes/print-mode";
|
||||
import type { PlanModeState } from "@oh-my-pi/pi-coding-agent/plan-mode/state";
|
||||
import type { AgentSession, AgentSessionEvent } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { type PlanProposalHandler, PROPOSE_DEVICE_NAME } from "@oh-my-pi/pi-coding-agent/tools/resolve";
|
||||
import type { PlanProposalHandler } from "@oh-my-pi/pi-coding-agent/tools/resolve";
|
||||
|
||||
function makeAssistantMessage(text: string): AssistantMessage {
|
||||
const timestamp = Date.now();
|
||||
@@ -177,51 +177,25 @@ describe("print mode working indicator", () => {
|
||||
vi.restoreAllMocks();
|
||||
});
|
||||
|
||||
it("enters default plan mode before submitting the initial prompt", async () => {
|
||||
const delayed = createDelayedSession(makeAssistantMessage("plan ready"), { defaultPlanMode: true });
|
||||
const run = runPrintMode(delayed.session, { mode: "text", initialMessage: "/plan hello" });
|
||||
it("does not enter startup plan mode in headless print mode and warns instead (#8272)", async () => {
|
||||
const delayed = createDelayedSession(makeAssistantMessage("final answer"), { defaultPlanMode: true });
|
||||
const run = runPrintMode(delayed.session, { mode: "text", initialMessage: "Reply with exactly: OK" });
|
||||
|
||||
await delayed.promptStarted;
|
||||
try {
|
||||
expect(delayed.getPlanModeAtPrompt()).toMatchObject({
|
||||
enabled: true,
|
||||
planFilePath: "local://PLAN.md",
|
||||
});
|
||||
expect(delayed.getModeChanges()).toEqual([{ mode: "plan", data: { planFilePath: "local://PLAN.md" } }]);
|
||||
const handler = delayed.getPlanProposalHandler();
|
||||
if (!handler) throw new Error("Expected print plan proposal handler");
|
||||
const proposal = await handler("hello");
|
||||
expect(proposal).toMatchObject({
|
||||
content: [{ type: "text", text: "Plan ready for review." }],
|
||||
details: { planFilePath: "local://hello-plan.md", title: "hello", planExists: true },
|
||||
});
|
||||
expect(delayed.getCurrentPlanMode()).toMatchObject({ planFilePath: "local://hello-plan.md" });
|
||||
expect(delayed.getModeChanges()).toEqual([
|
||||
{ mode: "plan", data: { planFilePath: "local://PLAN.md" } },
|
||||
{ mode: "plan", data: { planFilePath: "local://hello-plan.md" } },
|
||||
]);
|
||||
delayed.emit({
|
||||
type: "tool_execution_end",
|
||||
toolCallId: "proposal",
|
||||
toolName: "write",
|
||||
result: {
|
||||
content: proposal.content,
|
||||
details: {
|
||||
xdev: {
|
||||
tool: PROPOSE_DEVICE_NAME,
|
||||
mode: "execute",
|
||||
args: { title: "hello" },
|
||||
inner: proposal.details,
|
||||
},
|
||||
},
|
||||
},
|
||||
});
|
||||
await Promise.resolve();
|
||||
expect(delayed.getAbortCalls()).toBe(1);
|
||||
// Headless has no surface to review/approve/exit a plan, so the startup
|
||||
// default must not arm the plan-review flow — doing so stranded the turn
|
||||
// until the deadline (issue #8272).
|
||||
expect(delayed.getPlanModeAtPrompt()).toBeUndefined();
|
||||
expect(delayed.getModeChanges()).toEqual([]);
|
||||
expect(delayed.getPlanProposalHandler()).toBeUndefined();
|
||||
expect(stderrOutput.join("")).toContain("plan.defaultOnStartup is ignored in print mode");
|
||||
} finally {
|
||||
delayed.resolvePrompt();
|
||||
await run;
|
||||
}
|
||||
|
||||
expect(stdoutOutput.join("")).toBe("final answer\n");
|
||||
});
|
||||
|
||||
it("writes a text-mode working indicator before the prompt resolves and prints the final answer afterward", async () => {
|
||||
|
||||
Reference in New Issue
Block a user