From 7c96386e29747f6786ebd7680c7bba954e5c42a2 Mon Sep 17 00:00:00 2001 From: roboomp Date: Tue, 21 Jul 2026 21:31:12 +0000 Subject: [PATCH] fix(session): skipped stop hooks during aborts Short-circuited session_stop emission when an abort or disposal is already in progress, avoiding extension work whose result cannot be used. Added deterministic coverage for an abort racing the final settle pass. Fixes #6134 --- packages/coding-agent/CHANGELOG.md | 4 ++ .../src/prompts/system/workflow-notice.md | 6 +-- .../coding-agent/src/session/agent-session.ts | 4 ++ .../test/agent-session-concurrent.test.ts | 45 +++++++++++++++++++ 4 files changed, 54 insertions(+), 5 deletions(-) diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6f32dc885..1c4eba20a 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Fixed + +- Fixed in-progress aborts awaiting `session_stop` extension handlers whose results would be discarded ([#6134](https://github.com/can1357/oh-my-pi/issues/6134)). + ## [17.0.5] - 2026-07-18 ### Added diff --git a/packages/coding-agent/src/prompts/system/workflow-notice.md b/packages/coding-agent/src/prompts/system/workflow-notice.md index d15ad9127..09a849bef 100644 --- a/packages/coding-agent/src/prompts/system/workflow-notice.md +++ b/packages/coding-agent/src/prompts/system/workflow-notice.md @@ -47,7 +47,7 @@ For independent per-item chains (review → verify, fetch → extract → score) schema: FINDINGS_SCHEMA, }); return await parallel(found.findings.map((f) => async () => ({ - ...f, + …f, verdict: await agent( `Refute if you can (default refuted when unsure): ${f.title}`, { label: `verify:${f.file}`, schema: VERDICT_SCHEMA }, @@ -57,8 +57,6 @@ For independent per-item chains (review → verify, fetch → extract → score) phase("Review"); const results = await parallel(DIMENSIONS.map((d) => async () => reviewAndVerify(d))); const confirmed = results.flat().filter((f) => f.verdict.is_real); - - Reach for `pipeline()` only when a stage genuinely needs ALL of the previous stage first — dedup/merge across the whole set, early-exit on zero, or "compare against the other findings" — because its inter-stage barrier makes every item wait for the slowest peer: **Python (`eval`, Python backend):** @@ -80,8 +78,6 @@ Reach for `pipeline()` only when a stage genuinely needs ALL of the previous sta const verdicts = await parallel(findings.map((f) => async () => await agent(verifyPrompt(f), { schema: VERDICT_SCHEMA }), )); - - Use ordinary code between calls to flatten/map/filter; don't add a barrier just for that. Nested `parallel()` pools each cap independently, so keep total fan-out sane. diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index e39ca813d..e3248574c 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6381,6 +6381,10 @@ export class AgentSession { messages: AgentMessage[], lastAssistantMessage = this.getLastAssistantMessage(), ): Promise { + if (this.#abortInProgress || this.#isDisposed) { + this.#resetSessionStopContinuationState(); + return false; + } if (this.#agentKind === "sub" || !this.#extensionRunner?.hasHandlers("session_stop")) { return false; } diff --git a/packages/coding-agent/test/agent-session-concurrent.test.ts b/packages/coding-agent/test/agent-session-concurrent.test.ts index ef10ebae3..953b22f72 100644 --- a/packages/coding-agent/test/agent-session-concurrent.test.ts +++ b/packages/coding-agent/test/agent-session-concurrent.test.ts @@ -456,6 +456,51 @@ describe("AgentSession concurrent prompt guard", () => { ).toBe(true); }); + it("does not emit session_stop when abort starts before the settle pass", async () => { + const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; + const mock = createMockModel({ + handler: () => ({ content: ["Done"] }), + }); + const agent = new Agent({ + getApiKey: () => "test-key", + initialState: { model, systemPrompt: ["Test"], tools: [] }, + streamFn: mock.stream, + convertToLlm, + }); + const settleGate = Promise.withResolvers(); + const settleReached = Promise.withResolvers(); + const emitSessionStop = vi.fn().mockResolvedValue(undefined); + const extensionRunner = { + emit: vi.fn().mockResolvedValue(undefined), + emitBeforeAgentStart: vi.fn().mockResolvedValue(undefined), + hasHandlers: vi.fn((eventType: string) => eventType === "session_stop"), + emitSessionStop, + } as unknown as ExtensionRunner; + const sessionManager = SessionManager.inMemory(); + const settings = Settings.isolated(); + const authStorage = await AuthStorage.create(path.join(tempDir, "testauth.db")); + authStorages.push(authStorage); + const modelRegistry = new ModelRegistry(authStorage, path.join(tempDir, "models.yml")); + authStorage.setRuntimeApiKey("anthropic", "test-key"); + + session = new AgentSession({ agent, sessionManager, settings, modelRegistry, extensionRunner }); + vi.spyOn(session.goalRuntime, "onAgentEnd").mockImplementation(() => { + settleReached.resolve(); + return settleGate.promise; + }); + + const promptPromise = session.prompt("First message"); + await settleReached.promise; + const abortPromise = session.abort(); + settleGate.resolve(); + + await abortPromise; + await promptPromise; + await session.waitForIdle(); + + expect(emitSessionStop).not.toHaveBeenCalled(); + }); + it("does not continue session_stop feedback after aborting a slow hook", async () => { const model = getBundledModel("anthropic", "claude-sonnet-4-5")!; const mock = createMockModel({