diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 493bd5332..f94e3f821 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed `/new` starting an unsolicited old-context provider turn when a hidden steer (e.g. an `xd://` mount notice) was queued: the session transition is now an atomic boundary, so a queued steer/follow-up can no longer auto-resume against the pre-`/new` context while the session is disconnected mid-transition ([#5800](https://github.com/can1357/oh-my-pi/issues/5800)). + ## [17.0.2] - 2026-07-17 ### Added diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 945c7caa6..757581fda 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -2242,6 +2242,15 @@ export class AgentSession { * queue was consumed normally or a new turn already started. */ #drainStrandedQueuedMessages(): void { if (this.#abortInProgress) return; + // Session transitions (newSession/`/new`, compact, model-switch, session-switch, + // dispose) call #disconnectFromAgent() BEFORE `await abort()`, so abort's own + // finally lands here with no listener attached. Auto-resuming now would snapshot + // the still-old context (the transition hasn't reached agent.reset() yet), start a + // stale provider turn that races the reset, and — once reconnected — append its + // output to the fresh session (issue #5800). A disconnected session never owns the + // queue: the transition does. Leave any queued steer/follow-up for the post-transition + // state (reset drops them; an explicit prompt flushes them). + if (this.#unsubscribeAgent === undefined) return; // A concern steered into a resumed streaming run after a user interrupt can // strand at the turn tail (steered past the loop's final boundary poll). While // that interrupt's suppression is still in effect, reclaim such advisor steers diff --git a/packages/coding-agent/test/agent-session-new-session-queued-steer.test.ts b/packages/coding-agent/test/agent-session-new-session-queued-steer.test.ts new file mode 100644 index 000000000..beabc5f9a --- /dev/null +++ b/packages/coding-agent/test/agent-session-new-session-queued-steer.test.ts @@ -0,0 +1,142 @@ +import { afterEach, beforeEach, describe, expect, it } from "bun:test"; +import { Agent } from "@oh-my-pi/pi-agent-core"; +import { createMockModel, type MockModel } from "@oh-my-pi/pi-ai/providers/mock"; +import { getBundledModel } from "@oh-my-pi/pi-catalog/models"; +import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +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 { Snowflake, TempDir } from "@oh-my-pi/pi-utils"; + +const OLD_USER = "OLD_SESSION_USER_SENTINEL"; +const OLD_ASSISTANT = "OLD_SESSION_ASSISTANT_SENTINEL"; +const HIDDEN_XDEV = "HIDDEN_XDEV_STEER_SENTINEL"; +const LATE_OUTPUT = "LATE_OUTPUT_FROM_OLD_CONTINUE"; + +/** Collect the text of every message, tolerant of the AgentMessage union + * (some variants carry no `content`, and content blocks are a mixed union + * where only text blocks expose `text`). */ +function collectText(messages: readonly unknown[]): string[] { + const out: string[] = []; + for (const message of messages) { + if (!message || typeof message !== "object" || !("content" in message)) continue; + const content = message.content; + if (typeof content === "string") { + out.push(content); + continue; + } + if (!Array.isArray(content)) continue; + for (const block of content) { + if (block && typeof block === "object" && "type" in block && block.type === "text" && "text" in block) { + const text = block.text; + if (typeof text === "string") out.push(text); + } + } + } + return out; +} +describe("newSession() atomic boundary vs queued hidden steer", () => { + let tempDir: TempDir; + let session: AgentSession; + const authStorages: AuthStorage[] = []; + + beforeEach(() => { + tempDir = TempDir.createSync("@pi-new-session-steer-"); + }); + + afterEach(async () => { + try { + await session?.dispose(); + } finally { + for (const authStorage of authStorages.splice(0)) authStorage.close(); + await Bun.sleep(0); + await tempDir?.remove(); + } + }); + + it("does not start an old-context turn from a queued steer during /new", async () => { + const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; + const secondCallStarted = Promise.withResolvers(); + const releaseSecondCall = Promise.withResolvers(); + let secondRequestMessages: string[] = []; + let providerCalls = 0; + + const mock: MockModel = createMockModel({ + handler: async context => { + providerCalls++; + if (providerCalls === 1) { + return { content: [OLD_ASSISTANT], stopReason: "stop" }; + } + // Second (stale) turn: snapshot the request tail and hold the + // stream open so newSession() can complete before it appends. + secondRequestMessages = collectText(context.messages); + secondCallStarted.resolve(); + await releaseSecondCall.promise; + return { content: [LATE_OUTPUT], stopReason: "stop" }; + }, + }); + + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { model, systemPrompt: ["Test"], tools: [] }, + streamFn: mock.stream, + }); + const sessionManager = SessionManager.inMemory(); + const settings = Settings.isolated({ "compaction.enabled": false }); + const authStorage = await AuthStorage.create(tempDir.join(`auth-${Snowflake.next()}.db`)); + authStorages.push(authStorage); + authStorage.setRuntimeApiKey("anthropic", "test-key"); + const modelRegistry = new ModelRegistry(authStorage, tempDir.join("models.yml")); + session = new AgentSession({ agent, sessionManager, settings, modelRegistry }); + + // Build an old session with recognizable user + assistant messages. + await session.prompt(OLD_USER); + await agent.waitForIdle(); + expect(agent.state.messages.some(m => m.role === "assistant")).toBe(true); + + // Queue a hidden custom steer (xdev-mount-notice equivalent) while idle. + agent.steer({ + role: "custom", + customType: "xdev-mount-notice", + content: HIDDEN_XDEV, + display: false, + timestamp: Date.now(), + }); + expect(agent.hasQueuedMessages()).toBe(true); + + // Drive /new. The stale steer must NOT start an old-context turn. + const newSessionDone = session.newSession(); + + // Give the abort finally / post-prompt drain a chance to schedule. + const raced = await Promise.race([ + secondCallStarted.promise.then(() => "second-started" as const), + newSessionDone.then(() => "new-resolved" as const), + ]); + + const secondStartedBeforeReset = raced === "second-started"; + + await newSessionDone; + + // After the transition the fresh agent owns no queued messages: reset drops them. + expect(agent.hasQueuedMessages()).toBe(false); + + // Release the deferred stream (only relevant if a stale turn regressed into + // starting) and let any stream settle. + releaseSecondCall.resolve(); + await agent.waitForIdle(); + + const branchText = collectText(agent.state.messages); + + // Contract 1: no second provider request starts during newSession(). + expect(secondStartedBeforeReset).toBe(false); + expect(providerCalls).toBe(1); + // Contract 2: fresh branch contains no old markers or hidden steer. + expect(secondRequestMessages).not.toContain(OLD_USER); + expect(secondRequestMessages).not.toContain(HIDDEN_XDEV); + expect(branchText).not.toContain(OLD_USER); + expect(branchText).not.toContain(OLD_ASSISTANT); + // Contract 3: no late output appended to the fresh session. + expect(branchText).not.toContain(LATE_OUTPUT); + }); +});