Merge PR #8327: fix(vibe): cancel active turn on mode exit (@roboomp)
# Conflicts: # packages/coding-agent/test/interactive-mode-vibe-toggle.test.ts
This commit is contained in:
@@ -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
|
||||
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -601,6 +601,7 @@ export class AgentSession {
|
||||
#usageFallbackConfirmer: UsageFallbackConfirmer | undefined;
|
||||
#usagePreflightAbortControllers = new Set<AbortController>();
|
||||
#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<void>): Promise<void> {
|
||||
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;
|
||||
|
||||
@@ -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() },
|
||||
|
||||
@@ -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<void>();
|
||||
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<void>();
|
||||
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<void>();
|
||||
const releaseTeardown = Promise.withResolvers<void>();
|
||||
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");
|
||||
|
||||
Reference in New Issue
Block a user