From fbda6cfa9347d65808ec43adc28631a250c0d9cf Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 13 Jun 2026 21:27:51 +0000 Subject: [PATCH] fix(plan): /plan now exits paused state back to mode 'none' handlePlanModeCommand had branches for entering and pausing but fell through to #enterPlanMode() when planModePaused was true, so once a session entered plan mode it cycled forever between plan and plan_paused. /goal and other mode-gated commands were permanently blocked because they refuse to run while planModeEnabled || planModePaused is true. Add a paused branch that clears the paused flag, resets the reentry marker (so the next /plan logs as a fresh entry, not a resume), and appends a mode_change to "none". Tools, model, and plan state were already restored by the prior #exitPlanMode({ paused: true }), so no extra cleanup is needed. Fixes #2510 --- packages/coding-agent/CHANGELOG.md | 4 + .../src/modes/interactive-mode.ts | 13 +++ .../test/issue-2510-repro.test.ts | 98 +++++++++++++++++++ 3 files changed, 115 insertions(+) create mode 100644 packages/coding-agent/test/issue-2510-repro.test.ts diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 7fc96cb5e..a648f3f6d 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### 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 diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 221787c27..a4dd198e4 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -2237,6 +2237,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; diff --git a/packages/coding-agent/test/issue-2510-repro.test.ts b/packages/coding-agent/test/issue-2510-repro.test.ts new file mode 100644 index 000000000..1566ad36c --- /dev/null +++ b/packages/coding-agent/test/issue-2510-repro.test.ts @@ -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); + }); +});