diff --git a/docs/handoff-generation-pipeline.md b/docs/handoff-generation-pipeline.md index a2c91e1ce..bd2a10799 100644 --- a/docs/handoff-generation-pipeline.md +++ b/docs/handoff-generation-pipeline.md @@ -83,12 +83,12 @@ Capture is direct from the oneshot response; no agent-loop events or latest-assi ### 3) Cancellation checks -Cancellation throws `Error("Handoff cancelled")`; a completed generation with no text returns `undefined`. +An explicit user cancellation throws `Error("Handoff cancelled")`. Harness-initiated aborts preserve a supplied reason, or surface `Handoff aborted by session` when none is supplied. A manual handoff whose generation is empty/whitespace-only throws `Handoff generation produced no content`; auto-handoff returns `undefined` so maintenance can fall back to context-full compaction. -- caller signal aborts `#handoffAbortController` +- caller signal aborts `#handoffAbortController` and forwards its reason - `completeSimple(...)` receives the abort signal -- aborted handoff signal or provider `AbortError` is normalized to `Error("Handoff cancelled")` -- empty generated text returns `undefined` +- direct `abortHandoff()` or an unreasoned caller signal is normalized to `Error("Handoff cancelled")` +- harness abort reasons and provider failures (including provider `AbortError`s) surface verbatim `AgentSession.handoff()` always clears `#handoffAbortController` in `finally`. @@ -185,8 +185,8 @@ If auto generation returns no document, maintenance falls back to context-full c - appends `New session started with handoff context` - shows `savedPath` when the result includes one (manual `/handoff` normally has none) - On exception: - - if message is `"Handoff cancelled"` or error name is `AbortError`: `showError("Handoff cancelled")` - - otherwise: `showError("Handoff failed: ")` + - if message is `"Handoff cancelled"`: `showError("Handoff cancelled")` + - otherwise: logs the error and calls `showError("Handoff failed: ")` - Stops the loader, clears the status container, and requests render at end. Manual `/handoff` no longer streams the generated document into chat. A cancellable loader remains visible while the oneshot request runs, and the chat is rebuilt after generation completes. @@ -200,7 +200,7 @@ Manual `/handoff` no longer streams the generated document into chat. A cancella - `abortHandoff()` → aborts `#handoffAbortController` - `isGeneratingHandoff` → true while controller exists -When this abort path is used, the abort signal is passed to `completeSimple(...)`; `handoff()` normalizes the cancellation to `Error("Handoff cancelled")`, and command controller maps it to cancellation UI. +Direct `abortHandoff()` passes an unreasoned abort signal to `completeSimple(...)`; `handoff()` normalizes it to `Error("Handoff cancelled")`, and command controller maps it to cancellation UI. `AgentSession.abort(...)` instead aborts the handoff first with its harness reason (or `Handoff aborted by session`), so subsequent compaction cancellation cannot mask that failure as a user cancellation. ### Interactive `/handoff` path @@ -211,14 +211,14 @@ When this abort path is used, the abort signal is passed to `completeSimple(...) Current UI classification: - **Aborted/cancelled** - - `abortHandoff()` path triggers `"Handoff cancelled"`, or - - thrown `AbortError` + - direct `abortHandoff()` (interactive Esc) triggers `"Handoff cancelled"` + - an unreasoned caller signal also triggers `"Handoff cancelled"` - UI shows `Handoff cancelled` - **Failed** - - any other thrown error from the session transition or provider request path - - UI shows `Handoff failed: ...` + - a harness abort reason, an empty manual generation, or any thrown provider/session-transition error + - UI logs the error and shows `Handoff failed: ...` -Additional nuance: empty generated text or an extension-cancelled `session_before_switch` returns `undefined`, and the interactive controller currently reports **cancelled**, not **failed**. +An extension-cancelled `session_before_switch` returns `undefined`, which the interactive controller reports as **cancelled**. Empty generation is not an extension cancellation: manual handoff throws; auto-handoff returns `undefined` only for its context-full fallback. ## Short-session and minimum-content guardrails diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index f90cfdbad..b52535408 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -4,7 +4,7 @@ ### Fixed -- Fixed `/handoff` reporting "Handoff cancelled" for an empty or whitespace-only generation; a user-initiated handoff now surfaces "Handoff failed: Handoff generation produced no content" and logs the failure, while auto-handoff still falls back to context-full compaction ([#7993](https://github.com/can1357/oh-my-pi/issues/7993)). +- Fixed `/handoff` masking empty/whitespace-only generation and harness-initiated aborts as "Handoff cancelled"; manual empty generation now surfaces a logged failure, harness aborts preserve their reason (or report "Handoff aborted by session"), and auto-handoff still falls back to context-full compaction ([#7993](https://github.com/can1357/oh-my-pi/issues/7993)). ## [17.2.11] - 2026-08-07 diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index a2ae42c32..a92565c30 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -6219,6 +6219,9 @@ export class AgentSession { this.abortRetry(); this.#promptGeneration++; this.#scheduledHiddenNextTurnGeneration = undefined; + // Abort the handoff first so generic compaction cancellation cannot replace + // the harness reason with an unreasoned "Handoff cancelled". + this.#handoff.abortHandoff(new Error(options?.reason ?? "Handoff aborted by session")); if (options?.preserveCompaction) { // Manual `/compact` installed its own #compactionAbortController before // this internal abort and must keep it alive (that marker is what makes @@ -6230,7 +6233,6 @@ export class AgentSession { } else { this.abortCompaction(); } - this.abortHandoff(); this.abortBash(); this.abortEval(); const postPromptDrain = this.#cancelPostPromptTasks(); diff --git a/packages/coding-agent/src/session/session-handoff.ts b/packages/coding-agent/src/session/session-handoff.ts index 514c75373..662fa06e4 100644 --- a/packages/coding-agent/src/session/session-handoff.ts +++ b/packages/coding-agent/src/session/session-handoff.ts @@ -30,6 +30,17 @@ function createHandoffFileName(date = new Date()): string { return `handoff-${fileTimestamp}.md`; } +function throwIfHandoffAborted(signal: AbortSignal): void { + if (!signal.aborted) return; + const reason = signal.reason; + if (reason instanceof DOMException && reason.name === "AbortError") { + throw new Error("Handoff cancelled"); + } + if (reason instanceof Error) throw reason; + if (typeof reason === "string" && reason.length > 0) throw new Error(reason); + throw new Error("Handoff aborted by session"); +} + /** Capabilities borrowed from the owning AgentSession. */ export interface SessionHandoffHost { agent: Agent; @@ -81,10 +92,10 @@ export class SessionHandoff { this.#host = host; } /** - * Cancel in-progress handoff generation. + * Cancel in-progress handoff generation, preserving a harness-provided reason. */ - abortHandoff(): void { - this.#handoffAbortController?.abort(); + abortHandoff(reason?: Error): void { + this.#handoffAbortController?.abort(reason); } /** @@ -118,7 +129,7 @@ export class SessionHandoff { const sourceSignal = options?.signal; const onSourceAbort = () => { if (!handoffSignal.aborted) { - handoffAbortController.abort(); + handoffAbortController.abort(sourceSignal?.reason); } }; if (sourceSignal) { @@ -131,9 +142,7 @@ export class SessionHandoff { let advisorRecordersDetached = false; let sessionTransitioned = false; try { - if (handoffSignal.aborted) { - throw new Error("Handoff cancelled"); - } + throwIfHandoffAborted(handoffSignal); const model = this.#host.model(); if (!model) { @@ -208,9 +217,7 @@ export class SessionHandoff { ); const handoffText = this.#host.deobfuscateFromProvider(rawHandoffText); - if (handoffSignal.aborted) { - throw new Error("Handoff cancelled"); - } + throwIfHandoffAborted(handoffSignal); if (!handoffText || handoffText.trim().length === 0) { // Empty/whitespace-only generation is a real failure, not a user // cancellation. #7904 stopped masking provider errors as "Handoff @@ -324,13 +331,10 @@ export class SessionHandoff { return { document: handoffText, savedPath }; } catch (error) { - // Only a genuine abort (user Esc or the source turn cancelling) is a - // cancellation. A provider that throws a name==="AbortError" error without the - // handoff signal being aborted (stall/idle timeout, nested resolution failure) - // is a real failure and must surface verbatim, not be masked as "cancelled". - if (handoffSignal.aborted) { - throw new Error("Handoff cancelled"); - } + // Only a genuine cancellation (user Esc or an unreasoned source-signal + // abort) maps to "Handoff cancelled". A harness-provided abort reason and + // provider failures surface verbatim. + throwIfHandoffAborted(handoffSignal); throw error; } finally { if (advisorRecordersDetached) { diff --git a/packages/coding-agent/test/agent-session-handoff.test.ts b/packages/coding-agent/test/agent-session-handoff.test.ts index 06dd92570..e723cdcaf 100644 --- a/packages/coding-agent/test/agent-session-handoff.test.ts +++ b/packages/coding-agent/test/agent-session-handoff.test.ts @@ -2011,6 +2011,24 @@ describe("AgentSession handoff", () => { expect(generateHandoffSpy.mock.calls[0]?.[2]?.streamOptions?.signal?.aborted).toBe(true); }); + it("surfaces the reason when the harness aborts an in-flight handoff", async () => { + const started = Promise.withResolvers(); + const cancelled = Promise.withResolvers(); + vi.spyOn(compactionModule, "generateHandoffFromContext").mockImplementation((_context, _model, options) => { + started.resolve(); + options.streamOptions.signal?.addEventListener("abort", () => cancelled.reject(new Error("request aborted")), { + once: true, + }); + return cancelled.promise; + }); + + const handoffPromise = session.handoff(); + await started.promise; + await session.abort({ reason: "Harness stopped the session" }); + + await expect(handoffPromise).rejects.toThrow("Harness stopped the session"); + }); + it("surfaces the real error when generation fails without a user abort", async () => { // Providers throw name==="AbortError" errors on non-user conditions (stalls, // nested resolution failures). The handoff signal is never aborted here, so the