smashimo/oh-my-pi
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

fix(session): preserved harness handoff abort reasons

Browse Source 
Harness-initiated session aborts previously cancelled compaction before the handoff reason was recorded. The handoff catch then saw only an aborted signal and replaced the harness reason with "Handoff cancelled".

Abort the handoff first with the session reason, forward caller-signal reasons, and reserve "Handoff cancelled" for direct or unreasoned cancellation. Add a regression test for an in-flight handoff aborted through AgentSession.abort.

Fixes #7993
This commit is contained in:
roboomp
2026-08-08 09:02:21 +00:00
parent 92e574cb02
commit 7914e7c451
5 changed files with 55 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files
docs/handoff-generation-pipeline.md
+12 -12
View File
@@ -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: <message>")`
- if message is `"Handoff cancelled"`: `showError("Handoff cancelled")`
- otherwise: logs the error and calls `showError("Handoff failed: <message>")`
- 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
packages/coding-agent/CHANGELOG.md
+1 -1
View File
@@ -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
packages/coding-agent/src/session/agent-session.ts
+3 -1
View File
@@ -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();
packages/coding-agent/src/session/session-handoff.ts
+21 -17
View File
@@ -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) {
packages/coding-agent/test/agent-session-handoff.test.ts
+18
View File
@@ -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<void>();
const cancelled = Promise.withResolvers<string>();
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