From 8b1364d4c25c8ebde671a0887b34005ccfe6917f Mon Sep 17 00:00:00 2001 From: can1357 Date: Fri, 15 May 2026 18:49:53 +0200 Subject: [PATCH] feat(coding-agent): implemented one-shot generateHandoff in coding-agent - Replaced event-driven handoff with one-shot `generateHandoff(...)` via `completeSimple`. - Added cancellable `/handoff` command handling with a loader and Escape-to-abort flow. - Removed legacy `compaction/handoff.ts` exports and added `generateHandoff(messages, model, apiKey, options)`. - Fixed pre-cancelled handoff behavior to return `Handoff cancelled` and propagate abort signals. - Updated handoff tests/mocks to assert `generateHandoff` invocation details and `AgentSession.handoff()` options. --- docs/compaction.md | 10 +- docs/handoff-generation-pipeline.md | 168 ++++++++-------- packages/agent/CHANGELOG.md | 5 +- packages/agent/src/compaction/compaction.ts | 76 +++++++- packages/agent/src/compaction/handoff.ts | 44 ----- packages/agent/src/compaction/index.ts | 1 - packages/agent/test/handoff.test.ts | 104 +++++++--- packages/coding-agent/CHANGELOG.md | 5 +- .../modes/controllers/command-controller.ts | 27 ++- .../src/modes/controllers/event-controller.ts | 5 +- .../coding-agent/src/session/agent-session.ts | 115 +++++------ .../test/agent-session-handoff.test.ts | 184 ++++-------------- .../modes/controllers/handoff-command.test.ts | 79 ++++++++ 13 files changed, 464 insertions(+), 359 deletions(-) delete mode 100644 packages/agent/src/compaction/handoff.ts create mode 100644 packages/coding-agent/test/modes/controllers/handoff-command.test.ts diff --git a/docs/compaction.md b/docs/compaction.md index 35fb41db6..118254a65 100644 --- a/docs/compaction.md +++ b/docs/compaction.md @@ -9,12 +9,11 @@ Both are persisted as session entries and converted back into user-context messa ## Key implementation files -- `packages/agent/src/compaction/compaction.ts` +- `packages/agent/src/compaction/compaction.ts` (context-full summarization and handoff generation) - `packages/agent/src/compaction/branch-summarization.ts` - `packages/agent/src/compaction/pruning.ts` - `packages/agent/src/compaction/utils.ts` - `packages/agent/src/compaction/openai.ts` -- `packages/agent/src/compaction/handoff.ts` - `packages/coding-agent/src/session/session-manager.ts` - `packages/coding-agent/src/session/agent-session.ts` - `packages/coding-agent/src/session/messages.ts` @@ -198,6 +197,7 @@ Prompt selection: - iterative compaction with prior summary: `compaction-update-summary.md` - split-turn second pass: `compaction-turn-prefix.md` - short UI summary: `compaction-short-summary.md` +- handoff document: `handoff-document.md` (used by `generateHandoff(...)`, not serialized compaction) Remote summarization modes: @@ -206,6 +206,12 @@ Remote summarization modes: - Expects JSON containing at least `{ summary }`. - For OpenAI/OpenAI Codex models, compaction first tries the provider-native `/responses/compact` endpoint when remote compaction is enabled. It preserves provider replacement history in `preserveData.openaiRemoteCompaction` and falls back to local summarization if that native request fails. +### Handoff generation + +`packages/agent/src/compaction/compaction.ts` also exports `generateHandoff(...)`. Handoff generation uses the same `completeSimple(...)` oneshot style as summarization, but it preserves the live agent cache prefix by sending the active system prompt, tool array, and real LLM message history, then appending one agent-attributed `user` message containing the handoff prompt. It forces `toolChoice: "none"` and returns joined text blocks directly. + +Handoff does not write a `CompactionEntry`. `AgentSession.handoff()` owns the session transition: it starts a new session, injects the generated document as a visible `custom_message` with `customType: "handoff"`, and rebuilds agent messages from that new session. + ### File-operation context in summaries Compaction tracks cumulative file activity using assistant tool calls: diff --git a/docs/handoff-generation-pipeline.md b/docs/handoff-generation-pipeline.md index 0d93c1663..80b02568a 100644 --- a/docs/handoff-generation-pipeline.md +++ b/docs/handoff-generation-pipeline.md @@ -1,6 +1,6 @@ # `/handoff` generation pipeline -This document describes how the coding-agent implements `/handoff` today: trigger path, generation prompt, completion capture, session switch, and context reinjection. +This document describes how the coding-agent implements `/handoff`: trigger path, oneshot generation, session switch, context reinjection, persistence, and UI behavior. ## Scope @@ -8,7 +8,7 @@ Covers: - Interactive `/handoff` command dispatch - `AgentSession.handoff()` lifecycle and state transitions -- How handoff output is captured from assistant output +- `generateHandoff(...)` request shape - How old/new sessions persist handoff data differently - UI behavior for success, cancel, and failure @@ -22,7 +22,7 @@ Does not cover: - [`../src/modes/controllers/input-controller.ts`](../packages/coding-agent/src/modes/controllers/input-controller.ts) - [`../src/modes/controllers/command-controller.ts`](../packages/coding-agent/src/modes/controllers/command-controller.ts) - [`../src/session/agent-session.ts`](../packages/coding-agent/src/session/agent-session.ts) -- [`packages/agent/src/compaction/handoff.ts`](../packages/agent/src/compaction/handoff.ts) +- [`packages/agent/src/compaction/compaction.ts`](../packages/agent/src/compaction/compaction.ts) - [`../src/session/session-manager.ts`](../packages/coding-agent/src/session/session-manager.ts) - [`../src/extensibility/slash-commands.ts`](../packages/coding-agent/src/extensibility/slash-commands.ts) @@ -43,65 +43,83 @@ The same minimum-content guard exists again inside `AgentSession.handoff()` and `AgentSession.handoff(customInstructions?)`: -- Reads current branch entries (`sessionManager.getBranch()`) -- Validates minimum message count (`>= 2`) -- Creates `#handoffAbortController` -- Renders the fixed prompt template `packages/agent/src/compaction/prompts/handoff-document.md` via `renderHandoffPrompt(...)` with optional `additionalFocus` -- Appends `Additional focus: ...` if custom instructions are provided +- Reads current branch entries (`sessionManager.getBranch()`). +- Validates minimum message count (`>= 2`). +- Creates `#handoffAbortController` and links any caller-provided abort signal to it. +- Resolves the current model API key through `ModelRegistry`. +- Calls `generateHandoff(...)` with: + - live agent messages (`agent.state.messages`), + - the current model and API key, + - the base system prompt (`#baseSystemPrompt`), + - the live tool array (`agent.state.tools`), + - optional focus instructions, + - coding-agent message conversion (`convertToLlm`), + - provider metadata and `initiatorOverride: "agent"`. -Prompt is sent as an agent-authored developer message via: +`generateHandoff(...)` lives in `packages/agent/src/compaction/compaction.ts` next to summarization. It renders `packages/agent/src/compaction/prompts/handoff-document.md` via `renderHandoffPrompt(...)` with optional `additionalFocus`. + +### 2) Generate and capture output + +`generateHandoff(...)` converts the existing `AgentMessage[]` history to real LLM `Message[]` history, then appends one trailing agent-attributed `user` message containing the rendered handoff prompt. + +The request uses `completeSimple(...)` directly: ```ts -await this.#promptAgentWithIdleRetry([ +await completeSimple( + model, { - role: "developer", - content: [{ type: "text", text: handoffPrompt }], - attribution: "agent", - timestamp: Date.now(), + systemPrompt, + messages: requestMessages, + tools, }, -]); + { + apiKey, + signal, + reasoning: Effort.High, + toolChoice: "none", + initiatorOverride, + metadata, + }, +); ``` -Because handoff bypasses `prompt(...)`, normal slash/prompt-template expansion is not applied to this internal instruction payload. +Important generation properties: -### 2) Capture completion +- The request preserves the live provider cache prefix by reusing the same system prompt, tool definitions, and real message history shape as the active agent. +- The handoff instruction is a trailing `user` message, not a developer message, so the cached prefix remains aligned with the prior turn. +- `toolChoice: "none"` prevents intentional tool dispatch. +- The returned assistant content is filtered to text blocks and joined with `\n`; stray tool-call blocks are ignored if a provider does not honor `toolChoice: "none"`. +- `stopReason === "error"` throws a generation error. -Before sending prompt, `handoff()` subscribes to session events and waits for `agent_end`. - -On `agent_end`, it calls `extractHandoffDocument(...)`, which scans agent state backward for the most recent `assistant` message, then concatenates all `content` blocks where `type === "text"` with `\n`. - -Important extraction assumptions: - -- Only text blocks are used; non-text content is ignored. -- It assumes the latest assistant message corresponds to handoff generation. -- It does not parse markdown sections or validate format compliance. -- If assistant output has no text blocks, handoff is treated as missing. +No agent-loop events are used for capture. The handoff path no longer waits for `agent_end` and no longer scans the latest assistant message. ### 3) Cancellation checks -Cancellation throws `Error("Handoff cancelled")`; a completed generation with no extracted text returns `undefined`. +Cancellation throws `Error("Handoff cancelled")`; a completed generation with no text returns `undefined`. -- no captured handoff text → returns `undefined` -- aborted handoff signal → throws `Error("Handoff cancelled")` +- caller signal aborts `#handoffAbortController` +- `completeSimple(...)` receives the abort signal +- aborted handoff signal or provider `AbortError` is normalized to `Error("Handoff cancelled")` +- empty generated text returns `undefined` -It always clears `#handoffAbortController` in `finally`. +`AgentSession.handoff()` always clears `#handoffAbortController` in `finally`. ### 4) New session creation -If text was captured and not aborted: +If text was generated and not aborted: -1. Flush current session writer (`sessionManager.flush()`) -2. Cancel async jobs (`#asyncJobManager?.cancelAll()`) -3. Start a brand-new session (`sessionManager.newSession()`) -4. Reset in-memory agent state (`agent.reset()`) -5. Rebind `agent.sessionId` to new session id -6. Clear queued context arrays (`#steeringMessages`, `#followUpMessages`, `#pendingNextTurnMessages`) and any scheduled hidden next-turn generation -7. Reset todo reminder counter - `newSession()` creates a fresh header and empty entry list (leaf reset to `null`). In the handoff path, no `parentSession` is passed. +1. Flush current session writer (`sessionManager.flush()`). +2. Cancel session-owned async jobs. +3. Start a brand-new session with `parentSession` pointing at the previous session file when one exists. +4. Reset in-memory agent state (`agent.reset()`). +5. Rebind `agent.sessionId` to the new session id. +6. Rekey/reset hindsight state for the new session. +7. Clear queued context arrays (`#steeringMessages`, `#followUpMessages`, `#pendingNextTurnMessages`) and any scheduled hidden next-turn generation. +8. Reset todo reminder counter. ### 5) Handoff-context injection -The generated handoff document is wrapped by `createHandoffContext(...)` and appended to the new session as a `custom_message` entry: +The generated handoff document is wrapped by coding-agent session glue and appended to the new session as a `custom_message` entry: ```text @@ -114,23 +132,24 @@ The above is a handoff document from a previous session. Use this context to con Insertion call: ```ts -this.sessionManager.appendCustomMessageEntry("handoff", handoffContent, true); +this.sessionManager.appendCustomMessageEntry("handoff", handoffContent, true, undefined, "agent"); ``` Semantics: - `customType`: `"handoff"` - `display`: `true` (visible in TUI rebuild) +- attribution: `"agent"` - Entry type: `custom_message` (participates in LLM context) ### 6) Rebuild active agent context After injection: -1. `buildDisplaySessionContext()` resolves message list for current leaf -2. `agent.replaceMessages(sessionContext.messages)` makes the injected handoff message active context -3. Todo phases are synchronized from the new branch -4. Method returns `{ document: handoffText, savedPath? }` +1. `buildDisplaySessionContext()` resolves message list for current leaf. +2. `agent.replaceMessages(sessionContext.messages)` makes the injected handoff message active context. +3. Todo phases are synchronized from the new branch. +4. Method returns `{ document: handoffText, savedPath? }`. At this point, the active LLM context in the new session contains the injected handoff message, not the old transcript. @@ -138,9 +157,9 @@ At this point, the active LLM context in the new session contains the injected h ### Old session -During generation, normal message persistence remains active. The assistant handoff response is persisted as a regular `message` entry on `message_end`. +Handoff generation is a oneshot request, not a visible agent turn. The generated handoff text is not appended to the old session as an assistant message. -Result: the original session contains the visible generated handoff as part of historical transcript. +Result: the original session keeps its prior transcript unchanged except for data already persisted before handoff began. ### New session @@ -148,12 +167,15 @@ After session reset, handoff is persisted as `custom_message` with `customType: `buildSessionContext()` converts this entry into a runtime custom/user-context message via `createCustomMessage(...)`, so it is included in future prompts from the new session. +Auto-triggered handoffs can additionally write a timestamped `handoff-*.md` artifact under the session artifacts directory when `compaction.handoffSaveToDisk` is enabled. Manual `/handoff` does not write that artifact. + ## Controller/UI behavior `CommandController.handleHandoffCommand` behavior: -- Calls `await session.handoff(customInstructions)` -- If result is `undefined`: `showError("Handoff cancelled")` +- Shows a status loader: `Generating handoff… (esc to cancel)`. +- Calls `await session.handoff(customInstructions)`. +- If result is `undefined`: `showError("Handoff cancelled")`. - On success: - `rebuildChatFromMessages()` (loads new session context, including injected handoff) - invalidates status line and editor top border @@ -162,9 +184,11 @@ After session reset, handoff is persisted as `custom_message` with `customType: - On exception: - if message is `"Handoff cancelled"` or error name is `AbortError`: `showError("Handoff cancelled")` - otherwise: `showError("Handoff failed: ")` -- Requests render at end +- Stops the loader, restores the previous Escape handler, and requests render at end. -## Cancellation semantics (current behavior) +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. + +## Cancellation semantics ### Session-level cancellation primitive @@ -173,16 +197,11 @@ After session reset, handoff is persisted as `custom_message` with `customType: - `abortHandoff()` → aborts `#handoffAbortController` - `isGeneratingHandoff` → true while controller exists -When this abort path is used, the handoff completion waiter resolves as cancelled, `agent.abort()` is called, and `handoff()` throws `Error("Handoff cancelled")`; command controller maps it to cancellation UI. +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. ### Interactive `/handoff` path -The editor does not install a dedicated Escape handler that calls `abortHandoff()` for `/handoff`, but `InputController` treats `session.isGeneratingHandoff` as a busy state. - -Practical impact: - -- There is session-level cancellation support, but no handoff-specific keybinding hook in the `/handoff` command path. -- User interruption may still occur through broader agent abort paths, but that is not the same explicit cancellation channel used by `abortHandoff()`. +The command controller installs a temporary Escape handler for `/handoff` while the loader is visible. Pressing Escape calls `session.abortHandoff()`, which aborts the `completeSimple(...)` request through `#handoffAbortController`. ## Aborted vs failed handoff @@ -192,12 +211,11 @@ Current UI classification: - `abortHandoff()` path triggers `"Handoff cancelled"`, or - thrown `AbortError` - UI shows `Handoff cancelled` - - **Failed** - - any other thrown error from `handoff()` / prompt pipeline (model/API validation errors, runtime exceptions, etc.) + - any other thrown error from `handoff()` / `generateHandoff()` / provider request path - UI shows `Handoff failed: ...` -Additional nuance: if generation completes but no text is extracted, `handoff()` returns `undefined` and controller currently reports **cancelled**, not **failed**. +Additional nuance: if generation completes but no text is returned, `handoff()` returns `undefined` and controller currently reports **cancelled**, not **failed**. ## Short-session and minimum-content guardrails @@ -212,27 +230,25 @@ This avoids creating a new session with empty/near-empty handoff context. High-level state flow: -1. Interactive slash command intercepted -2. Preflight message-count guard -3. `#handoffAbortController` created (`isGeneratingHandoff = true`) -4. Internal developer handoff prompt submitted (visible in chat as normal assistant generation) -5. On `agent_end`, last assistant text extracted -6. If missing text → return `undefined`; if aborted → cancellation error path +1. Interactive slash command intercepted. +2. Preflight message-count guard. +3. `#handoffAbortController` created (`isGeneratingHandoff = true`). +4. `generateHandoff(...)` issues one `completeSimple(...)` request with live system prompt, tools, message history, and trailing handoff prompt. +5. Assistant response text blocks are joined; tool-call blocks are discarded. +6. If missing text → return `undefined`; if aborted → cancellation error path. 7. If present: - flush old session - cancel async jobs - - create new empty session + - create new empty session with previous session as parent - reset runtime queues/counters - append `custom_message(handoff)` - optionally save an auto-triggered handoff document under the session artifacts directory when `compaction.handoffSaveToDisk` is enabled -8. Controller rebuilds chat UI and announces success -9. `#handoffAbortController` cleared (`isGeneratingHandoff = false`) +8. Controller rebuilds chat UI and announces success. +9. `#handoffAbortController` cleared (`isGeneratingHandoff = false`). ## Known assumptions and limitations -- Handoff extraction is heuristic: "last assistant text blocks"; no structural validation. -- No hard check that generated markdown follows requested section format. -- Missing extracted text is reported as cancellation in controller UX. -- `/handoff` interactive flow currently lacks a dedicated Escape→`abortHandoff()` binding, though the session reports handoff as a busy state. -- New session lineage metadata (`parentSession`) is not set by this path. +- No structural validation checks that generated markdown follows the requested section format. +- Missing generated text is reported as cancellation in controller UX. +- Manual handoff has no streaming visibility; a cancellable loader is shown until the UI updates after generation completes. - Auto-triggered handoffs can write a timestamped `handoff-*.md` artifact when `compaction.handoffSaveToDisk` is enabled; write failure is logged and does not fail the handoff. diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index 98329c6ae..f0a2c84d6 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -1,13 +1,15 @@ # Changelog ## [Unreleased] - ### Breaking Changes +- Removed the `@oh-my-pi/pi-agent-core/compaction/handoff` exports from the package surface, including `extractHandoffDocument`, `createHandoffContext`, and `createHandoffFileName` - Removed legacy telemetry constants from the public enum surface (including `AGGREGATE_ATTR`, `GenAIAttr.System`, and old `gen_ai.*` extension keys such as `gen_ai.request.service_tier`/cost/tool status/handoff fields) and replaced them with `OpenAIAttr`, `PiGenAIAttr`, and `PiGenAIAggregateAttr` ### Added +- Added `generateHandoff(messages, model, apiKey, options)` to `@oh-my-pi/pi-agent-core/compaction` to generate a handoff document by calling the model directly, using live system/tool context and optional metadata +- Added generation filtering so the returned handoff document now includes only text content blocks from the model output - Added support for defining `AgentTool` schemas with Zod, with legacy TypeBox schemas still supported when generating tool schemas for model calls - Added `OpenAIAttr`, `PiGenAIAttr`, and `PiGenAIAggregateAttr` exports so consumers can reference the new `openai.*` and `pi.gen_ai.*` telemetry attribute keys directly - Added `onChatUsage` to `AgentTelemetryConfig`, an always-fired hook receiving a `ChatUsageEvent` for every chat step that produced usage. The event carries the chat `span`, `agent`, `conversationId`, `stepNumber`, `model`, `provider`, `serviceTier`, `usage`, optional `cost`, and resolved dynamic `attributes` — independent of whether a `costEstimator` is configured. @@ -27,6 +29,7 @@ ### Changed +- Changed handoff document generation to force `toolChoice: "none"` when calling the model so tool invocation is disabled during generation - Changed `chat` spans to emit normalized provider identifiers in `gen_ai.provider.name` via OTEL-style values (for example `google` to `gcp.gemini`) instead of the legacy `gen_ai.system` label - Changed service-tier telemetry to emit `openai.request.service_tier`/`openai.response.service_tier` only when supported by provider via `shouldSendServiceTier`, rather than always using `gen_ai.request.service_tier` - Changed captured message payloads so full capture now records OTEL-structured message parts with `pi.gen_ai.request.messages`, `pi.gen_ai.system_instructions`, and `gen_ai.output.messages` including assistant `finish_reason` diff --git a/packages/agent/src/compaction/compaction.ts b/packages/agent/src/compaction/compaction.ts index 8f9166191..a6da65769 100644 --- a/packages/agent/src/compaction/compaction.ts +++ b/packages/agent/src/compaction/compaction.ts @@ -9,13 +9,14 @@ import { type AssistantMessage, completeSimple, Effort, + type Message, type MessageAttribution, type Model, type Usage, } from "@oh-my-pi/pi-ai"; import { countTokens } from "@oh-my-pi/pi-natives"; import { logger, prompt } from "@oh-my-pi/pi-utils"; -import type { AgentMessage } from "../types"; +import type { AgentMessage, AgentTool } from "../types"; import type { CompactionEntry, SessionEntry } from "./entries"; import { type ConvertToLlm, convertToLlm, createBranchSummaryMessage, createCustomMessage } from "./messages"; import { @@ -26,10 +27,12 @@ import { shouldUseOpenAiRemoteCompaction, withOpenAiRemoteCompactionPreserveData, } from "./openai"; +import autoHandoffThresholdFocusPrompt from "./prompts/auto-handoff-threshold-focus.md" with { type: "text" }; import compactionShortSummaryPrompt from "./prompts/compaction-short-summary.md" with { type: "text" }; import compactionSummaryPrompt from "./prompts/compaction-summary.md" with { type: "text" }; import compactionTurnPrefixPrompt from "./prompts/compaction-turn-prefix.md" with { type: "text" }; import compactionUpdateSummaryPrompt from "./prompts/compaction-update-summary.md" with { type: "text" }; +import handoffDocumentPrompt from "./prompts/handoff-document.md" with { type: "text" }; import { computeFileLists, @@ -489,6 +492,10 @@ const UPDATE_SUMMARIZATION_PROMPT = prompt.render(compactionUpdateSummaryPrompt) const SHORT_SUMMARY_PROMPT = prompt.render(compactionShortSummaryPrompt); +const HANDOFF_DOCUMENT_PROMPT = prompt.render(handoffDocumentPrompt); + +export const AUTO_HANDOFF_THRESHOLD_FOCUS = prompt.render(autoHandoffThresholdFocusPrompt); + function formatAdditionalContext(context: string[] | undefined): string { if (!context || context.length === 0) return ""; const lines = context.map(line => `- ${line}`).join("\n"); @@ -588,6 +595,73 @@ export async function generateSummary( return textContent; } +// ============================================================================ +// Handoff generation +// ============================================================================ + +export interface HandoffOptions { + /** Live agent system prompt — passed verbatim so providers hit the cached prefix. */ + systemPrompt: string[]; + /** Live agent tool list — same purpose. Forced to `toolChoice: "none"`. */ + tools?: AgentTool[]; + customInstructions?: string; + convertToLlm?: ConvertToLlm; + initiatorOverride?: MessageAttribution; + metadata?: Record; +} + +export function renderHandoffPrompt(customInstructions?: string): string { + if (!customInstructions) return HANDOFF_DOCUMENT_PROMPT; + return prompt.render(handoffDocumentPrompt, { + additionalFocus: customInstructions, + }); +} + +export async function generateHandoff( + messages: AgentMessage[], + model: Model, + apiKey: string, + options: HandoffOptions, + signal?: AbortSignal, +): Promise { + const llmMessages = (options.convertToLlm ?? convertToLlm)(messages); + const requestMessages: Message[] = [ + ...llmMessages, + { + role: "user", + content: [{ type: "text", text: renderHandoffPrompt(options.customInstructions) }], + attribution: "agent", + timestamp: Date.now(), + }, + ]; + + const response = await completeSimple( + model, + { + systemPrompt: options.systemPrompt, + messages: requestMessages, + tools: options.tools, + }, + { + apiKey, + signal, + reasoning: Effort.High, + toolChoice: "none", + initiatorOverride: options.initiatorOverride, + metadata: options.metadata, + }, + ); + + if (response.stopReason === "error") { + throw new Error(`Handoff generation failed: ${response.errorMessage || "Unknown error"}`); + } + + return response.content + .filter((c): c is { type: "text"; text: string } => c.type === "text") + .map(c => c.text) + .join("\n"); +} + async function generateShortSummary( recentMessages: AgentMessage[], historySummary: string | undefined, diff --git a/packages/agent/src/compaction/handoff.ts b/packages/agent/src/compaction/handoff.ts deleted file mode 100644 index f40ce61aa..000000000 --- a/packages/agent/src/compaction/handoff.ts +++ /dev/null @@ -1,44 +0,0 @@ -import type { AssistantMessage } from "@oh-my-pi/pi-ai"; -import { prompt } from "@oh-my-pi/pi-utils"; -import type { AgentMessage } from "../types"; -import autoHandoffThresholdFocusPrompt from "./prompts/auto-handoff-threshold-focus.md" with { type: "text" }; -import handoffDocumentPrompt from "./prompts/handoff-document.md" with { type: "text" }; - -/** Result from a handoff operation. */ -export interface HandoffResult { - document: string; - savedPath?: string; -} - -export interface HandoffOptions { - autoTriggered?: boolean; - signal?: AbortSignal; -} - -export const AUTO_HANDOFF_THRESHOLD_FOCUS = prompt.render(autoHandoffThresholdFocusPrompt); - -export function renderHandoffPrompt(customInstructions?: string): string { - return prompt.render(handoffDocumentPrompt, { - additionalFocus: customInstructions, - }); -} - -export function extractHandoffDocument(messages: AgentMessage[]): string | undefined { - for (let i = messages.length - 1; i >= 0; i--) { - const message = messages[i]; - if (message.role !== "assistant") continue; - const content = (message as AssistantMessage).content; - const textParts = content.filter((c): c is { type: "text"; text: string } => c.type === "text").map(c => c.text); - if (textParts.length > 0) return textParts.join("\n"); - } - return undefined; -} - -export function createHandoffContext(document: string): string { - return `\n${document}\n\n\nThe above is a handoff document from a previous session. Use this context to continue the work seamlessly.`; -} - -export function createHandoffFileName(date = new Date()): string { - const fileTimestamp = date.toISOString().replace(/[:.]/g, "-"); - return `handoff-${fileTimestamp}.md`; -} diff --git a/packages/agent/src/compaction/index.ts b/packages/agent/src/compaction/index.ts index 7ca4f9812..d993b002d 100644 --- a/packages/agent/src/compaction/index.ts +++ b/packages/agent/src/compaction/index.ts @@ -6,7 +6,6 @@ export * from "./branch-summarization"; export * from "./compaction"; export * from "./entries"; export * from "./errors"; -export * from "./handoff"; export * from "./messages"; export * from "./openai"; export * from "./pruning"; diff --git a/packages/agent/test/handoff.test.ts b/packages/agent/test/handoff.test.ts index 0add25d8d..911536093 100644 --- a/packages/agent/test/handoff.test.ts +++ b/packages/agent/test/handoff.test.ts @@ -1,17 +1,15 @@ -import { describe, expect, test } from "bun:test"; -import { - AUTO_HANDOFF_THRESHOLD_FOCUS, - createHandoffContext, - createHandoffFileName, - extractHandoffDocument, - renderHandoffPrompt, -} from "@oh-my-pi/pi-agent-core/compaction/handoff"; -import type { AssistantMessage } from "@oh-my-pi/pi-ai"; +import { afterEach, describe, expect, test, vi } from "bun:test"; +import type { AgentMessage, AgentTool } from "@oh-my-pi/pi-agent-core"; +import { AUTO_HANDOFF_THRESHOLD_FOCUS, generateHandoff, renderHandoffPrompt } from "@oh-my-pi/pi-agent-core/compaction"; +import type { AssistantMessage, Model, ToolCall } from "@oh-my-pi/pi-ai"; +import * as ai from "@oh-my-pi/pi-ai"; +import { Effort } from "@oh-my-pi/pi-ai"; +import { getBundledModel } from "@oh-my-pi/pi-ai/models"; -function assistantText(text: string): AssistantMessage { +function createAssistantMessage(content: AssistantMessage["content"]): AssistantMessage { return { role: "assistant", - content: [{ type: "text", text }], + content, timestamp: Date.now(), provider: "mock", model: "mock", @@ -28,6 +26,18 @@ function assistantText(text: string): AssistantMessage { }; } +function getTestModel(): Model { + const model = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!model) { + throw new Error("Expected built-in anthropic model to exist"); + } + return model; +} + +afterEach(() => { + vi.restoreAllMocks(); +}); + describe("handoff helpers", () => { test("renders custom focus into the handoff prompt", () => { const rendered = renderHandoffPrompt("preserve failing test name"); @@ -41,20 +51,64 @@ describe("handoff helpers", () => { ); }); - test("extracts the latest assistant text document", () => { - const document = extractHandoffDocument([ - { role: "user", content: "older", timestamp: 1 }, - assistantText("old handoff"), - { role: "user", content: "newer", timestamp: 2 }, - assistantText("new handoff"), - ]); - expect(document).toBe("new handoff"); - }); + test("generates handoff with the live cache prefix and tool use disabled", async () => { + const strayToolCall: ToolCall = { type: "toolCall", id: "call_1", name: "read", arguments: {} }; + const completeSimpleSpy = vi + .spyOn(ai, "completeSimple") + .mockResolvedValue( + createAssistantMessage([ + { type: "text", text: "## Goal\nContinue" }, + strayToolCall, + { type: "text", text: "## Next Steps\n1. Run the focused test" }, + ]), + ); + const model = getTestModel(); + const systemPrompt = ["Live system prompt"]; + const tools: AgentTool[] = []; + const messages: AgentMessage[] = [ + { role: "user", content: "start work", timestamp: 1 }, + createAssistantMessage([{ type: "text", text: "started" }]), + ]; - test("creates the persisted handoff context and filename", () => { - expect(createHandoffContext("## Goal\nContinue")).toBe( - "\n## Goal\nContinue\n\n\nThe above is a handoff document from a previous session. Use this context to continue the work seamlessly.", - ); - expect(createHandoffFileName(new Date("2026-05-15T12:34:56.789Z"))).toBe("handoff-2026-05-15T12-34-56-789Z.md"); + const document = await generateHandoff(messages, model, "test-key", { + systemPrompt, + tools, + customInstructions: "preserve failing test name", + initiatorOverride: "agent", + metadata: { session: "handoff-test" }, + }); + + expect(document).toBe("## Goal\nContinue\n## Next Steps\n1. Run the focused test"); + expect(completeSimpleSpy).toHaveBeenCalledTimes(1); + const call = completeSimpleSpy.mock.calls[0]; + if (!call) throw new Error("Expected completeSimple call"); + const [calledModel, context, options] = call; + expect(calledModel).toBe(model); + expect(context.systemPrompt).toBe(systemPrompt); + expect(context.tools).toBe(tools); + expect(context.messages[0]).toMatchObject({ role: "user", content: "start work" }); + expect(options).toMatchObject({ + apiKey: "test-key", + reasoning: Effort.High, + toolChoice: "none", + initiatorOverride: "agent", + metadata: { session: "handoff-test" }, + }); + + const lastMessage = context.messages[context.messages.length - 1]; + if (!lastMessage) throw new Error("Expected trailing handoff prompt message"); + if (lastMessage.role !== "user") { + throw new Error("Expected trailing handoff prompt to be a user message"); + } + expect(lastMessage.attribution).toBe("agent"); + if (!Array.isArray(lastMessage.content)) { + throw new Error("Expected handoff prompt content blocks"); + } + const promptBlock = lastMessage.content[0]; + if (promptBlock?.type !== "text") { + throw new Error("Expected text handoff prompt block"); + } + expect(promptBlock.text).toContain("Write a handoff document"); + expect(promptBlock.text).toContain("Additional focus: preserve failing test name"); }); }); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 567385afa..095b8ed1c 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -1,13 +1,13 @@ # Changelog ## [Unreleased] - ### Breaking Changes - Changed the extension and hook runtime API by moving schema typing from direct TypeBox imports to `TSchema` from `@oh-my-pi/pi-ai`, requiring callers who use TypeScript imports of `Type` to migrate via provided injected modules ### Added +- Added a cancellable handoff progress indicator in `/handoff` that displays while handoff generation runs and can be aborted with `Esc` - Added `apiKey` as a supported provider override field in model config, allowing API-key-only overrides to provide fallback credentials for built-in models - Added `supportsMultipleSystemMessages`, `allowsSyntheticReasoningContentForToolCalls`, `disableReasoningOnToolChoice`, and `levels` model-thinking compatibility fields to model configuration schemas - Added `zod` to the Extension, Custom Tool, Hook, and Custom Command APIs as `pi.zod` so extension and plugin authors can define tool schemas with Zod without separate imports @@ -16,6 +16,7 @@ ### Changed +- Changed handoff generation to run as a one-shot handoff request and switch to the new session only after it completes, avoiding an extra assistant handoff turn in chat history - Changed `pi.typebox.Type.Composite` to merge all object schemas in the provided list, enabling more than two object inputs - Changed `pi.typebox.Type.Record` to validate record keys against the provided key schema instead of forcing string keys - Changed `pi.typebox.Type.Array` with `uniqueItems: true` to reject duplicate items while preserving the constraint in wire schemas @@ -33,6 +34,8 @@ ### Fixed +- Fixed auto-triggered handoff flow to perform only a single handoff-generation model call instead of an extra prompt cycle +- Fixed handoff cancellation behavior so a pre-cancelled signal returns `Handoff cancelled` without starting generation and aborting handoff now propagates through the handoff request signal - Fixed `create_conventional_analysis` parsing to ignore harmless extra fields and still parse the required conventional fields - Fixed BashTool async request validation flow so async execution remains disabled and returns the explicit `Async bash execution is disabled` error - Fixed `task.simple` invalid `schema` and `context` argument handling to still reject unsupported fields after tool-argument validation diff --git a/packages/coding-agent/src/modes/controllers/command-controller.ts b/packages/coding-agent/src/modes/controllers/command-controller.ts index ead2e8746..3cf7424fc 100644 --- a/packages/coding-agent/src/modes/controllers/command-controller.ts +++ b/packages/coding-agent/src/modes/controllers/command-controller.ts @@ -1175,8 +1175,29 @@ export class CommandController { return; } + if (this.ctx.loadingAnimation) { + this.ctx.loadingAnimation.stop(); + this.ctx.loadingAnimation = undefined; + } + this.ctx.statusContainer.clear(); + + const originalOnEscape = this.ctx.editor.onEscape; + this.ctx.editor.onEscape = () => { + this.ctx.session.abortHandoff(); + }; + + const handoffLoader = new Loader( + this.ctx.ui, + spinner => theme.fg("accent", spinner), + text => theme.fg("muted", text), + "Generating handoff… (esc to cancel)", + getSymbolTheme().spinnerFrames, + ); + this.ctx.statusContainer.addChild(handoffLoader); + this.ctx.ui.requestRender(); + try { - // The agent will visibly generate the handoff document in chat + // Handoff generation runs as a oneshot request; the new session is shown after it completes. const result = await this.ctx.session.handoff(customInstructions); if (!result) { @@ -1206,6 +1227,10 @@ export class CommandController { } else { this.ctx.showError(`Handoff failed: ${message}`); } + } finally { + handoffLoader.stop(); + this.ctx.statusContainer.clear(); + this.ctx.editor.onEscape = originalOnEscape; } this.ctx.ui.requestRender(); } diff --git a/packages/coding-agent/src/modes/controllers/event-controller.ts b/packages/coding-agent/src/modes/controllers/event-controller.ts index 6e6521318..7d1d1bdba 100644 --- a/packages/coding-agent/src/modes/controllers/event-controller.ts +++ b/packages/coding-agent/src/modes/controllers/event-controller.ts @@ -719,9 +719,8 @@ export class EventController { #scheduleIdleCompaction(): void { this.#cancelIdleCompaction(); - // Don't schedule while compaction/handoff is already running — the agent_end from a - // handoff agent turn still has the old session's bloated token counts, and scheduling - // here would fire after the session resets, trying to handoff an empty session. + // Don't schedule idle work while context maintenance is already running; the + // maintenance flow may reset the session before this timer fires. if (this.ctx.session.isCompacting) return; const idleSettings = settings.getGroup("compaction"); diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index d77dbac30..039c2e518 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -35,15 +35,10 @@ import { calculatePromptTokens, collectEntriesForBranchSummary, compact, - createHandoffContext, - createHandoffFileName, estimateTokens, - extractHandoffDocument, generateBranchSummary, - type HandoffOptions, - type HandoffResult, + generateHandoff, prepareCompaction, - renderHandoffPrompt, type SummaryOptions, shouldCompact, } from "@oh-my-pi/pi-agent-core/compaction"; @@ -339,6 +334,17 @@ export interface PromptOptions { skipCompactionCheck?: boolean; } +/** Result from a handoff operation. */ +export interface HandoffResult { + document: string; + savedPath?: string; +} + +export interface SessionHandoffOptions { + autoTriggered?: boolean; + signal?: AbortSignal; +} + /** Result from cycleModel() */ export interface ModelCycleResult { model: Model; @@ -504,6 +510,15 @@ const noOpUIContext: ExtensionUIContext = { setToolsExpanded: () => {}, }; +function createHandoffContext(document: string): string { + return `\n${document}\n\n\nThe above is a handoff document from a previous session. Use this context to continue the work seamlessly.`; +} + +function createHandoffFileName(date = new Date()): string { + const fileTimestamp = date.toISOString().replace(/[:.]/g, "-"); + return `handoff-${fileTimestamp}.md`; +} + // ============================================================================ // ACP Permission Gate // ============================================================================ @@ -5128,16 +5143,13 @@ export class AgentSession { } /** - * Generate a handoff document by asking the agent, then start a new session with it. - * - * This prompts the current agent to write a comprehensive handoff document, - * waits for completion, then starts a fresh session with the handoff as context. + * Generate a handoff document with a oneshot LLM call, then start a new session with it. * * @param customInstructions Optional focus for the handoff document * @param options Handoff execution options * @returns The handoff document text, or undefined if cancelled/failed */ - async handoff(customInstructions?: string, options?: HandoffOptions): Promise { + async handoff(customInstructions?: string, options?: SessionHandoffOptions): Promise { const entries = this.sessionManager.getBranch(); const messageCount = entries.filter(e => e.type === "message").length; @@ -5151,10 +5163,6 @@ export class AgentSession { const handoffAbortController = this.#handoffAbortController; const handoffSignal = handoffAbortController.signal; const sourceSignal = options?.signal; - const onHandoffAbort = () => { - this.agent.abort(); - }; - handoffSignal.addEventListener("abort", onHandoffAbort, { once: true }); const onSourceAbort = () => { if (!handoffSignal.aborted) { handoffAbortController.abort(); @@ -5167,55 +5175,36 @@ export class AgentSession { } } - // Build the handoff prompt - const handoffPrompt = renderHandoffPrompt(customInstructions); - - // Create a promise that resolves when the agent completes - let handoffText: string | undefined; - const { promise: completionPromise, resolve: resolveCompletion } = Promise.withResolvers(); - let handoffCancelled = false; - let unsubscribe: (() => void) | undefined; - const onCompletionAbort = () => { - unsubscribe?.(); - handoffCancelled = true; - resolveCompletion(); - }; - if (handoffSignal.aborted) { - onCompletionAbort(); - } else { - handoffSignal.addEventListener("abort", onCompletionAbort, { once: true }); - } - unsubscribe = this.subscribe(event => { - if (event.type === "agent_end") { - unsubscribe?.(); - handoffSignal.removeEventListener("abort", onCompletionAbort); - handoffText = extractHandoffDocument(this.agent.state.messages); - resolveCompletion(); - } - }); - try { - // Send the prompt and wait for completion if (handoffSignal.aborted) { throw new Error("Handoff cancelled"); } - this.#beginInFlight(); - try { - this.agent.setSystemPrompt(this.#baseSystemPrompt); - await this.#promptAgentWithIdleRetry([ - { - role: "developer", - content: [{ type: "text", text: handoffPrompt }], - attribution: "agent", - timestamp: Date.now(), - }, - ]); - } finally { - this.#endInFlight(); - } - await completionPromise; - if (handoffCancelled || handoffSignal.aborted) { + const model = this.model; + if (!model) { + throw new Error("No model selected for handoff"); + } + const apiKey = await this.#modelRegistry.getApiKey(model, this.sessionId); + if (!apiKey) { + throw new Error(`No API key for ${model.provider}`); + } + + const handoffText = await generateHandoff( + this.agent.state.messages, + model, + apiKey, + { + systemPrompt: this.#baseSystemPrompt, + tools: this.agent.state.tools, + customInstructions, + convertToLlm, + initiatorOverride: "agent", + metadata: this.agent.metadataForProvider(model.provider), + }, + handoffSignal, + ); + + if (handoffSignal.aborted) { throw new Error("Handoff cancelled"); } if (!handoffText) { @@ -5266,10 +5255,12 @@ export class AgentSession { this.#syncTodoPhasesFromBranch(); return { document: handoffText, savedPath }; + } catch (error) { + if (handoffSignal.aborted || (error instanceof Error && error.name === "AbortError")) { + throw new Error("Handoff cancelled"); + } + throw error; } finally { - unsubscribe?.(); - handoffSignal.removeEventListener("abort", onCompletionAbort); - handoffSignal.removeEventListener("abort", onHandoffAbort); sourceSignal?.removeEventListener("abort", onSourceAbort); this.#handoffAbortController = undefined; } diff --git a/packages/coding-agent/test/agent-session-handoff.test.ts b/packages/coding-agent/test/agent-session-handoff.test.ts index fede2b5d4..a2b7b0977 100644 --- a/packages/coding-agent/test/agent-session-handoff.test.ts +++ b/packages/coding-agent/test/agent-session-handoff.test.ts @@ -1,6 +1,7 @@ import { afterEach, beforeEach, describe, expect, it, vi } from "bun:test"; import * as path from "node:path"; import { Agent } from "@oh-my-pi/pi-agent-core"; +import * as compactionModule from "@oh-my-pi/pi-agent-core/compaction"; import type { AssistantMessage, ToolCall } from "@oh-my-pi/pi-ai"; import { getBundledModel } from "@oh-my-pi/pi-ai/models"; import { createMockModel } from "@oh-my-pi/pi-ai/providers/mock"; @@ -92,40 +93,13 @@ describe("AgentSession handoff", () => { }); it("does not run auto-compaction after handoff turn completes", async () => { - const model = session.model; - if (!model) { - throw new Error("Expected model to be set"); - } - const handoffText = "## Goal\nContinue from here"; - const handoffAssistant: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: handoffText }], - api: model.api, - provider: model.provider, - model: model.id, - stopReason: "stop", - usage: { - input: 190_000, - output: 1_000, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 191_000, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }, - timestamp: Date.now(), - }; - - const promptSpy = vi.spyOn(session.agent, "prompt").mockImplementation(async () => { - session.agent.replaceMessages([handoffAssistant]); - session.agent.emitExternalEvent({ type: "message_end", message: handoffAssistant }); - session.agent.emitExternalEvent({ type: "agent_end", messages: [handoffAssistant] }); - }); + const generateHandoffSpy = vi.spyOn(compactionModule, "generateHandoff").mockResolvedValue(handoffText); const result = await session.handoff(); await Bun.sleep(20); - expect(promptSpy).toHaveBeenCalledTimes(1); + expect(generateHandoffSpy).toHaveBeenCalledTimes(1); expect(result?.document).toBe(handoffText); expect(events.filter(event => event.type === "auto_compaction_start")).toHaveLength(0); expect(events.filter(event => event.type === "auto_compaction_end")).toHaveLength(0); @@ -192,35 +166,8 @@ describe("AgentSession handoff", () => { throw new Error("Expected previous session file"); } - const model = session.model; - if (!model) { - throw new Error("Expected model to be set"); - } - const handoffText = "## Goal\nContinue from here"; - const handoffAssistant: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: handoffText }], - api: model.api, - provider: model.provider, - model: model.id, - stopReason: "stop", - usage: { - input: 1, - output: 1, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 2, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }, - timestamp: Date.now(), - }; - - vi.spyOn(session.agent, "prompt").mockImplementation(async () => { - session.agent.replaceMessages([handoffAssistant]); - session.agent.emitExternalEvent({ type: "message_end", message: handoffAssistant }); - session.agent.emitExternalEvent({ type: "agent_end", messages: [handoffAssistant] }); - }); + vi.spyOn(compactionModule, "generateHandoff").mockResolvedValue(handoffText); const result = await session.handoff(); const handoffSessionFile = session.sessionFile; @@ -441,18 +388,6 @@ describe("AgentSession handoff", () => { cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, }, }, - { - content: [{ type: "text", text: "## Goal\nContinue from here" }], - stopReason: "stop", - usage: { - input: 8_000, - output: 500, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 8_500, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }, - }, ], }); @@ -483,9 +418,13 @@ describe("AgentSession handoff", () => { events.push(event); }); + const generateHandoffSpy = vi + .spyOn(compactionModule, "generateHandoff") + .mockResolvedValue("## Goal\nContinue from here"); await session.prompt("Trigger threshold handoff"); - expect(mock.calls).toHaveLength(2); + expect(mock.calls).toHaveLength(1); + expect(generateHandoffSpy).toHaveBeenCalledTimes(1); const endEvents = events.filter(event => event.type === "auto_compaction_end"); expect(endEvents).toHaveLength(1); expect(endEvents[0]).toMatchObject({ type: "auto_compaction_end", action: "handoff", aborted: false }); @@ -563,7 +502,7 @@ describe("AgentSession handoff", () => { vi.spyOn(extensionRunner, "emit").mockResolvedValue(undefined); const mock = createMockModel({ - responses: [{ content: ["normal response"] }, { content: ["## Goal\nContinue from here"] }], + responses: [{ content: ["normal response"] }], }); const agent = new Agent({ getApiKey: () => "test-key", @@ -607,44 +546,23 @@ describe("AgentSession handoff", () => { }); await session.prompt("hello from user"); + const generateHandoffSpy = vi + .spyOn(compactionModule, "generateHandoff") + .mockResolvedValue("## Goal\nContinue from here"); await session.handoff(); expect(emitBeforeAgentStart).toHaveBeenCalledTimes(1); - expect(mock.calls.map(c => c.context.systemPrompt?.join("\n\n") ?? "")).toEqual(["Hook override", "Test"]); + expect(mock.calls.map(c => c.context.systemPrompt?.join("\n\n") ?? "")).toEqual(["Hook override"]); + const handoffCall = generateHandoffSpy.mock.calls[0]; + if (!handoffCall) throw new Error("Expected generateHandoff call"); + expect(handoffCall[3].systemPrompt).toEqual(["Test"]); }); it("saves auto-handoff document to disk when enabled", async () => { session.settings.set("compaction.handoffSaveToDisk", true); - const model = session.model; - if (!model) { - throw new Error("Expected model to be set"); - } - const handoffText = "## Goal\nContinue from here"; - const handoffAssistant: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: handoffText }], - api: model.api, - provider: model.provider, - model: model.id, - stopReason: "stop", - usage: { - input: 190_000, - output: 1_000, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 191_000, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }, - timestamp: Date.now(), - }; - - vi.spyOn(session.agent, "prompt").mockImplementation(async () => { - session.agent.replaceMessages([handoffAssistant]); - session.agent.emitExternalEvent({ type: "message_end", message: handoffAssistant }); - session.agent.emitExternalEvent({ type: "agent_end", messages: [handoffAssistant] }); - }); + vi.spyOn(compactionModule, "generateHandoff").mockResolvedValue(handoffText); const result = await session.handoff(undefined, { autoTriggered: true }); expect(result?.savedPath).toBeDefined(); @@ -657,34 +575,7 @@ describe("AgentSession handoff", () => { it("does not save manual handoff document when save setting is enabled", async () => { session.settings.set("compaction.handoffSaveToDisk", true); - const model = session.model; - if (!model) { - throw new Error("Expected model to be set"); - } - - const handoffAssistant: AssistantMessage = { - role: "assistant", - content: [{ type: "text", text: "## Goal\nManual handoff" }], - api: model.api, - provider: model.provider, - model: model.id, - stopReason: "stop", - usage: { - input: 190_000, - output: 1_000, - cacheRead: 0, - cacheWrite: 0, - totalTokens: 191_000, - cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, - }, - timestamp: Date.now(), - }; - - vi.spyOn(session.agent, "prompt").mockImplementation(async () => { - session.agent.replaceMessages([handoffAssistant]); - session.agent.emitExternalEvent({ type: "message_end", message: handoffAssistant }); - session.agent.emitExternalEvent({ type: "agent_end", messages: [handoffAssistant] }); - }); + vi.spyOn(compactionModule, "generateHandoff").mockResolvedValue("## Goal\nManual handoff"); const result = await session.handoff(); expect(result?.savedPath).toBeUndefined(); @@ -694,30 +585,39 @@ describe("AgentSession handoff", () => { const controller = new AbortController(); controller.abort(); - const promptSpy = vi.spyOn(session.agent, "prompt"); - const abortSpy = vi.spyOn(session.agent, "abort"); + const generateHandoffSpy = vi.spyOn(compactionModule, "generateHandoff"); await expect(session.handoff(undefined, { signal: controller.signal })).rejects.toThrow("Handoff cancelled"); - expect(promptSpy).not.toHaveBeenCalled(); - expect(abortSpy).toHaveBeenCalledTimes(1); + expect(generateHandoffSpy).not.toHaveBeenCalled(); }); it("aborts handoff generation when provided signal is cancelled", async () => { const controller = new AbortController(); - const { promise: promptPromise, resolve: resolvePrompt } = Promise.withResolvers(); - const promptSpy = vi.spyOn(session.agent, "prompt").mockImplementation(async () => { - await promptPromise; - }); - const abortSpy = vi.spyOn(session.agent, "abort").mockImplementation(() => { - resolvePrompt(); - }); + const started = Promise.withResolvers(); + const cancelled = Promise.withResolvers(); + const generateHandoffSpy = vi + .spyOn(compactionModule, "generateHandoff") + .mockImplementation((_messages, _model, _apiKey, _options, signal) => { + started.resolve(); + const onAbort = () => { + const error = new Error("aborted"); + error.name = "AbortError"; + cancelled.reject(error); + }; + if (signal?.aborted) { + onAbort(); + } else { + signal?.addEventListener("abort", onAbort, { once: true }); + } + return cancelled.promise; + }); const handoffPromise = session.handoff(undefined, { signal: controller.signal }); - await Bun.sleep(10); + await started.promise; controller.abort(); await expect(handoffPromise).rejects.toThrow("Handoff cancelled"); - expect(promptSpy).toHaveBeenCalledTimes(1); - expect(abortSpy).toHaveBeenCalled(); + expect(generateHandoffSpy).toHaveBeenCalledTimes(1); + expect(generateHandoffSpy.mock.calls[0]?.[4]?.aborted).toBe(true); }); }); diff --git a/packages/coding-agent/test/modes/controllers/handoff-command.test.ts b/packages/coding-agent/test/modes/controllers/handoff-command.test.ts new file mode 100644 index 000000000..73fa4c437 --- /dev/null +++ b/packages/coding-agent/test/modes/controllers/handoff-command.test.ts @@ -0,0 +1,79 @@ +import { afterEach, beforeAll, describe, expect, it, vi } from "bun:test"; +import { CommandController } from "@oh-my-pi/pi-coding-agent/modes/controllers/command-controller"; +import { getThemeByName, setThemeInstance } from "@oh-my-pi/pi-coding-agent/modes/theme/theme"; +import type { InteractiveModeContext } from "@oh-my-pi/pi-coding-agent/modes/types"; + +function createContainer() { + return { + children: [] as unknown[], + addChild(child: unknown) { + this.children.push(child); + }, + clear() { + this.children = []; + }, + }; +} + +describe("/handoff command", () => { + beforeAll(async () => { + const theme = await getThemeByName("dark"); + if (!theme) throw new Error("Expected dark theme"); + setThemeInstance(theme); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("shows a cancellable loader while handoff generation is running", async () => { + const handoffStarted = Promise.withResolvers(); + const handoffDone = Promise.withResolvers<{ document: string }>(); + const originalOnEscape = vi.fn(); + const statusContainer = createContainer(); + const chatContainer = createContainer(); + const abortHandoff = vi.fn(); + const requestRender = vi.fn(); + const ctx = { + sessionManager: { + getEntries: () => [{ type: "message" }, { type: "message" }], + }, + session: { + handoff: vi.fn(() => { + handoffStarted.resolve(); + return handoffDone.promise; + }), + abortHandoff, + }, + loadingAnimation: undefined, + statusContainer, + chatContainer, + ui: { requestRender }, + editor: { onEscape: originalOnEscape }, + rebuildChatFromMessages: vi.fn(), + statusLine: { invalidate: vi.fn() }, + updateEditorTopBorder: vi.fn(), + updateEditorBorderColor: vi.fn(), + reloadTodos: vi.fn(async () => undefined), + showStatus: vi.fn(), + showWarning: vi.fn(), + showError: vi.fn(), + } as unknown as InteractiveModeContext; + const controller = new CommandController(ctx); + + const commandPromise = controller.handleHandoffCommand("focus on tests"); + await handoffStarted.promise; + + expect(statusContainer.children).toHaveLength(1); + expect(ctx.editor.onEscape).not.toBe(originalOnEscape); + ctx.editor.onEscape?.(); + expect(abortHandoff).toHaveBeenCalledTimes(1); + + handoffDone.resolve({ document: "## Goal\nContinue" }); + await commandPromise; + + expect(statusContainer.children).toHaveLength(0); + expect(ctx.editor.onEscape).toBe(originalOnEscape); + expect(ctx.session.handoff).toHaveBeenCalledWith("focus on tests"); + }); +});