Merge PR #2513: /plan exits paused state back to mode none
This commit is contained in:
@@ -68,6 +68,10 @@
|
||||
|
||||
- Fixed `AgentBusyError` ("Agent is already processing. Use steer() or followUp()...") surfacing on mode transitions — as `Failed to finalize approved plan: ...` when a plan was approved while the agent was still streaming the post-`resolve` continuation (or a turn started by the approve-time compaction/clear), and as an error toast when a loop auto-submit or goal continuation fired during a streaming/compaction race. Plan approval now aborts any in-flight turn before dispatching the executor's first prompt, and `submitInteractiveInput` routes streaming-time loop, goal-continuation, and manual submissions through the follow-up queue (`streamingBehavior: "followUp"`) instead of throwing (synthetic continue-shortcuts stay developer-attributed and keep their prior behavior). Extends the manual-`/goal` fix in [#2454](https://github.com/can1357/oh-my-pi/issues/2454) to the continuation and plan-approval paths.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `/plan` cycling between `plan` and `plan_paused` with no path back to mode `none`. `handlePlanModeCommand` had branches for entering and pausing but fell through to `#enterPlanMode()` when invoked from the paused state, so once a session entered plan mode the only operator-visible toggle re-entered it. The handler now matches `planModePaused` and fully exits — clearing `planModeHasEntered` and appending a `mode_change` to `"none"` — so `/goal` (and any other mode gated on `planModeEnabled || planModePaused`) can run again after a third `/plan` ([#2510](https://github.com/can1357/oh-my-pi/issues/2510)).
|
||||
|
||||
## [15.12.5] - 2026-06-13
|
||||
### Changed
|
||||
|
||||
|
||||
@@ -2309,6 +2309,19 @@ export class InteractiveMode implements InteractiveModeContext {
|
||||
await this.#exitPlanMode({ paused: true });
|
||||
return;
|
||||
}
|
||||
if (this.planModePaused) {
|
||||
// Third toggle: paused → off. Tools, model, and plan state were already
|
||||
// restored by the prior #exitPlanMode({ paused: true }); only the
|
||||
// paused flag, the reentry marker, and the session mode entry remain.
|
||||
// Without this branch the handler fell through to #enterPlanMode and
|
||||
// the session was stuck cycling plan ↔ plan_paused (issue #2510).
|
||||
this.planModePaused = false;
|
||||
this.#planModeHasEntered = false;
|
||||
this.#updatePlanModeStatus();
|
||||
this.sessionManager.appendModeChange("none");
|
||||
this.showStatus("Plan mode disabled.");
|
||||
return;
|
||||
}
|
||||
if (!this.session.settings.get("plan.enabled")) {
|
||||
this.showWarning("Plan mode is disabled. Enable it in settings (plan.enabled).");
|
||||
return;
|
||||
|
||||
@@ -0,0 +1,98 @@
|
||||
import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test";
|
||||
import * as path from "node:path";
|
||||
import { Agent } from "@oh-my-pi/pi-agent-core";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { resetSettingsForTest, Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { InteractiveMode } from "@oh-my-pi/pi-coding-agent/modes/interactive-mode";
|
||||
import { initTheme } from "@oh-my-pi/pi-coding-agent/modes/theme/theme";
|
||||
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 { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/**
|
||||
* Issue #2510 — `/plan` only cycled between `plan` and `plan_paused`, never
|
||||
* returning to `none`. That left `/goal` and any other mode-gated command
|
||||
* permanently blocked because `planModePaused` stayed true.
|
||||
*
|
||||
* Contract: three consecutive `/plan` invocations from a fresh session must
|
||||
* land on `enabled=false, paused=false` and append a `mode_change` to `"none"`.
|
||||
*/
|
||||
describe("issue #2510 — /plan toggles plan → plan_paused → none", () => {
|
||||
let tempDir: TempDir;
|
||||
let authStorage: AuthStorage;
|
||||
let session: AgentSession;
|
||||
let mode: InteractiveMode;
|
||||
|
||||
beforeAll(() => {
|
||||
initTheme();
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
resetSettingsForTest();
|
||||
tempDir = TempDir.createSync("@pi-issue-2510-");
|
||||
await Settings.init({ inMemory: true, cwd: tempDir.path() });
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db"));
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
const defaultModel = modelRegistry.find("anthropic", "claude-sonnet-4-5");
|
||||
if (!defaultModel) throw new Error("Expected claude-sonnet-4-5 in registry");
|
||||
|
||||
session = new AgentSession({
|
||||
agent: new Agent({
|
||||
initialState: {
|
||||
model: defaultModel,
|
||||
systemPrompt: ["Test"],
|
||||
tools: [],
|
||||
messages: [],
|
||||
},
|
||||
}),
|
||||
sessionManager: SessionManager.create(tempDir.path(), tempDir.path()),
|
||||
settings: Settings.isolated(),
|
||||
modelRegistry,
|
||||
});
|
||||
mode = new InteractiveMode(session, "test");
|
||||
});
|
||||
|
||||
afterEach(async () => {
|
||||
vi.restoreAllMocks();
|
||||
mode?.stop();
|
||||
await session?.dispose();
|
||||
authStorage?.close();
|
||||
tempDir?.removeSync();
|
||||
resetSettingsForTest();
|
||||
});
|
||||
|
||||
it("third /plan returns the session to mode 'none' instead of re-entering plan", async () => {
|
||||
// First /plan → enter plan mode.
|
||||
await mode.handlePlanModeCommand();
|
||||
expect(mode.planModeEnabled).toBe(true);
|
||||
expect(mode.planModePaused).toBe(false);
|
||||
expect(session.sessionManager.buildSessionContext().mode).toBe("plan");
|
||||
|
||||
// Second /plan → pause (PLAN.md is empty so no confirm prompt).
|
||||
await mode.handlePlanModeCommand();
|
||||
expect(mode.planModeEnabled).toBe(false);
|
||||
expect(mode.planModePaused).toBe(true);
|
||||
expect(session.sessionManager.buildSessionContext().mode).toBe("plan_paused");
|
||||
|
||||
// Third /plan → fully disable. Pre-fix this re-entered plan mode and the
|
||||
// session cycled forever between plan ↔ plan_paused.
|
||||
await mode.handlePlanModeCommand();
|
||||
expect(mode.planModeEnabled).toBe(false);
|
||||
expect(mode.planModePaused).toBe(false);
|
||||
expect(session.sessionManager.buildSessionContext().mode).toBe("none");
|
||||
});
|
||||
|
||||
it("after exiting through paused, /plan starts a fresh plan session (not a re-entry)", async () => {
|
||||
await mode.handlePlanModeCommand(); // enter
|
||||
await mode.handlePlanModeCommand(); // pause
|
||||
await mode.handlePlanModeCommand(); // off — clears the reentry marker
|
||||
|
||||
await mode.handlePlanModeCommand();
|
||||
expect(mode.planModeEnabled).toBe(true);
|
||||
// `reentry` mirrors `#planModeHasEntered`. The fresh /plan after a full
|
||||
// exit should look like a first entry, not a resume, so plan-mode prompts
|
||||
// don't read "you're back in plan mode" on what is logically a new run.
|
||||
expect(session.getPlanModeState()?.reentry).toBe(false);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user