diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index ec863da50..5f6291db0 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -185,6 +185,9 @@ - Fixed retry-fallback selection switching to a fallback model with a context window too small to hold the current session context. - Fixed OpenCode discovery ignoring `opencode.jsonc` files and rejecting comments in `opencode.json`. - Fixed WSL2 startup hanging forever when the Windows interop pipe is wedged: the WSL host-home discovery probes (`cmd.exe`, `wslpath`) now run under a 500ms hard timeout and fall back to the Linux `$HOME`/`~/.omp` candidates ([#8402](https://github.com/can1357/oh-my-pi/issues/8402)). +### Fixed + +- Fixed `/vibe` cancellation leaving an in-flight model turn unaware that Vibe mode and its tools were removed ([#8326](https://github.com/can1357/oh-my-pi/issues/8326)). ## [17.2.15] - 2026-08-12 diff --git a/packages/coding-agent/src/modes/interactive-mode.ts b/packages/coding-agent/src/modes/interactive-mode.ts index 87f73a538..ec6eb9c6b 100644 --- a/packages/coding-agent/src/modes/interactive-mode.ts +++ b/packages/coding-agent/src/modes/interactive-mode.ts @@ -3531,10 +3531,18 @@ export class InteractiveMode implements InteractiveModeContext { if (!this.vibeModeEnabled) { return; } - const ownerScope = this.#vibeModeOwnerScope; - const killed = await VibeSessionRegistry.global().killAll(this.#vibeParentSession(), ownerScope); - await this.session.deactivateVibeTools(this.#vibeModePreviousTools ?? []); - this.session.setVibeModeState(undefined); + // Tear down with the queued-message drain suppressed: aborting the active + // turn would otherwise let a queued user steer/follow-up restart on the + // still-live Vibe tools before this teardown removes them (issue #8326). + let killed = 0; + await this.session.runModeExitTeardown(async () => { + if (this.session.isStreaming) { + await this.session.abort(); + } + killed = await VibeSessionRegistry.global().killAll(this.#vibeParentSession(), this.#vibeModeOwnerScope); + await this.session.deactivateVibeTools(this.#vibeModePreviousTools ?? []); + this.session.setVibeModeState(undefined); + }); this.vibeModeEnabled = false; this.#vibeModePreviousTools = undefined; this.#vibeModeOwnerScope = undefined; diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 8b825c0b2..e6a5dc3b0 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -601,6 +601,7 @@ export class AgentSession { #usageFallbackConfirmer: UsageFallbackConfirmer | undefined; #usagePreflightAbortControllers = new Set(); #queuedMessageDrainBlocked = false; + #modeExitDrainSuppressionDepth = 0; #usagePreflightReadyForNextModelCall = false; #usagePreflightReadyModel: Model | undefined; #detachUsageBeforeQueueDequeue: (() => void) | undefined; @@ -5946,6 +5947,32 @@ export class AgentSession { this.#scheduleIdleQueueDrain(); } + /** + * Run a mode-exit `teardown` (abort the active turn, swap the toolset, clear + * mode state) with queued-message auto-resume suppressed, then re-arm the + * drain so a queued user turn resumes cleanly once the previous toolset is + * back. + * + * `abort()`'s stranded-queue drain runs from its own `finally`; without this + * guard a queued steer/follow-up behind the aborted turn would start a fresh + * `agent.continue()` during the teardown's `await`s — while the exiting mode's + * tools/context are still live — and then have those tools removed underneath + * it, reintroducing the mode's stale-tool failure on the restarted turn + * (issue #8326). Suppressing the drain across teardown guarantees the queued + * turn resumes only after teardown, so it runs as a clean non-mode turn. + */ + async runModeExitTeardown(teardown: () => Promise): Promise { + this.#modeExitDrainSuppressionDepth++; + try { + await teardown(); + } finally { + this.#modeExitDrainSuppressionDepth--; + if (this.#modeExitDrainSuppressionDepth === 0) { + this.#scheduleIdleQueueDrain(); + } + } + } + async #queueUserMessage( text: string, images: ImageContent[] | undefined, @@ -5994,6 +6021,7 @@ export class AgentSession { #scheduleQueuedMessageDrain(): void { if ( this.#queuedMessageDrainScheduled || + this.#modeExitDrainSuppressionDepth > 0 || this.#queuedMessageDrainBlocked || !this.#canAutoContinueForFollowUp() || !this.agent.hasQueuedMessages() @@ -6004,7 +6032,11 @@ export class AgentSession { this.#scheduleAgentContinue({ shouldContinue: () => { this.#queuedMessageDrainScheduled = false; - return this.#canAutoContinueForFollowUp() && this.agent.hasQueuedMessages(); + return ( + this.#modeExitDrainSuppressionDepth === 0 && + this.#canAutoContinueForFollowUp() && + this.agent.hasQueuedMessages() + ); }, onSkip: () => { this.#queuedMessageDrainScheduled = false; diff --git a/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts b/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts index fb0fb5a65..f0b7b4f76 100644 --- a/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts +++ b/packages/coding-agent/test/agent-session-steer-idle-drain.test.ts @@ -112,6 +112,23 @@ describe("AgentSession steer idle drain", () => { expect(continueSpy).toHaveBeenCalledTimes(1); }); + it("delivers successive idle steers after each successful drain", async () => { + await createSession([{ role: "user", content: "hello", timestamp: Date.now() }, createAssistantMessage()]); + const continueSpy = vi.spyOn(session.agent, "continue").mockImplementation(async () => { + session.agent.clearAllQueues(); + }); + + await session.steer("first steer"); + vi.advanceTimersByTime(200); + await session.waitForIdle(); + + await session.steer("second steer"); + vi.advanceTimersByTime(200); + await session.waitForIdle(); + + expect(continueSpy).toHaveBeenCalledTimes(2); + }); + it("delivers a steer queued after an interrupted tool result", async () => { await createSession([ { role: "user", content: "hello", timestamp: Date.now() }, diff --git a/packages/coding-agent/test/interactive-mode-vibe-toggle.test.ts b/packages/coding-agent/test/interactive-mode-vibe-toggle.test.ts index 0e5e17537..c1620e65c 100644 --- a/packages/coding-agent/test/interactive-mode-vibe-toggle.test.ts +++ b/packages/coding-agent/test/interactive-mode-vibe-toggle.test.ts @@ -10,7 +10,8 @@ import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { type } from "@oh-my-pi/omptype"; -import { Agent, type AgentTool } from "@oh-my-pi/pi-agent-core"; +import { Agent, type AgentTool, type StreamFn } from "@oh-my-pi/pi-agent-core"; +import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; 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"; @@ -23,7 +24,7 @@ import { VIBE_TOOL_NAMES } from "@oh-my-pi/pi-coding-agent/tools/vibe"; import { EventBus } from "@oh-my-pi/pi-coding-agent/utils/event-bus"; import { VibeSessionRegistry } from "@oh-my-pi/pi-coding-agent/vibe/runtime"; import { TempDir } from "@oh-my-pi/pi-utils"; -import { createInMemoryAuthStorage } from "./helpers/agent-session-setup"; +import { createAssistantMessage, createInMemoryAuthStorage } from "./helpers/agent-session-setup"; function stubTool(name: string): AgentTool { return { @@ -82,6 +83,7 @@ describe("InteractiveMode vibe mode toggle", () => { let tempDir: TempDir; let authStorage: AuthStorage; let session: AgentSession; + let streamFn: StreamFn | undefined; let mode: InteractiveMode; let modelRegistry: ModelRegistry; let storage: ExitFaultStorage; @@ -110,6 +112,10 @@ describe("InteractiveMode vibe mode toggle", () => { tools: [], messages: [], }, + streamFn: (...args) => { + if (!streamFn) throw new Error("No test stream configured"); + return streamFn(...args); + }, }), sessionManager: SessionManager.create(tempDir.path(), tempDir.path(), storage), settings: Settings.isolated({}), @@ -157,6 +163,93 @@ describe("InteractiveMode vibe mode toggle", () => { expect(session.getAllToolNames().toSorted()).toEqual(["read", "todo"]); }); + it("cancels an in-flight model turn before removing Vibe tools", async () => { + const started = Promise.withResolvers(); + streamFn = (_model, _context, options) => { + const stream = new AssistantMessageEventStream(); + queueMicrotask(() => { + stream.push({ type: "start", partial: createAssistantMessage("") }); + options?.signal?.addEventListener( + "abort", + () => stream.push({ type: "error", reason: "aborted", error: createAssistantMessage("Aborted") }), + { once: true }, + ); + started.resolve(); + }); + return stream; + }; + await mode.handleVibeModeCommand(); + const prompt = session.prompt("Delegate this"); + await started.promise; + expect(session.isStreaming).toBe(true); + + await mode.handleVibeModeCommand(); + await prompt; + + expect(session.isStreaming).toBe(false); + expect(session.getToolByName("vibe_spawn")).toBeUndefined(); + }); + + it("holds a user steer queued during Vibe teardown until the tools are removed", async () => { + const toolNamesPerCall: string[][] = []; + const firstStarted = Promise.withResolvers(); + streamFn = (_model, context, options) => { + toolNamesPerCall.push((context.tools ?? []).map(tool => tool.name)); + const isFirst = toolNamesPerCall.length === 1; + const stream = new AssistantMessageEventStream(); + queueMicrotask(() => { + stream.push({ type: "start", partial: createAssistantMessage("") }); + if (isFirst) { + options?.signal?.addEventListener( + "abort", + () => stream.push({ type: "error", reason: "aborted", error: createAssistantMessage("Aborted") }), + { once: true }, + ); + firstStarted.resolve(); + } else { + stream.push({ type: "done", reason: "stop", message: createAssistantMessage("Resumed") }); + } + }); + return stream; + }; + + await mode.handleVibeModeCommand(); + const prompt = session.prompt("Delegate this"); + await firstStarted.promise; + + const abortSettled = Promise.withResolvers(); + const releaseTeardown = Promise.withResolvers(); + const abort = session.abort.bind(session); + vi.spyOn(session, "abort").mockImplementation(async options => { + await abort(options); + abortSettled.resolve(); + await releaseTeardown.promise; + }); + const exit = mode.handleVibeModeCommand(); + await abortSettled.promise; + // Queue while teardown is still guarded. The regular queue path clears its + // retry block, but must not clear the independent mode-exit suppression. + await session.steer("and then do the other thing"); + // Drain the microtasks in which an unguarded schedule calls + // agent.continue(). The queued steer must remain owned by the queue until + // teardown releases. + for (let index = 0; index < 5; index++) await Promise.resolve(); + expect(session.agent.peekSteeringQueue()).toHaveLength(1); + expect(toolNamesPerCall.length).toBe(1); + releaseTeardown.resolve(); + + await exit; + await prompt; + await session.waitForIdle(); + + expect(toolNamesPerCall.length).toBe(2); + for (const name of VIBE_TOOL_NAMES) { + expect(toolNamesPerCall[1]).not.toContain(name); + } + expect(session.getVibeModeState()).toBeUndefined(); + expect(session.getToolByName("vibe_spawn")).toBeUndefined(); + }); + it("keeps a same-named non-built-in Todo tool unavailable in Vibe mode", async () => { const model = session.model; if (!model) throw new Error("Expected active model");