fix(agent): retry handoff, branch summary and manual /compact on a blip
Each is a single, side-effect-free completion whose result is parsed after it resolves, so one transient provider failure previously aborted the whole operation - for /compact that left the user's context full. Adds SummaryOptions.oneshotRetry, because both compaction paths call the same generateSummary and the policy therefore cannot be a constant inside it. Manual /compact has no outer loop and gets retry by default; auto-compaction passes false because session-maintenance already retries the whole attempt, and nesting would multiply the budget (10 outer x 3 inner) while stacking each outer wait on an inner backoff.
This commit is contained in:
@@ -19,6 +19,14 @@
|
||||
### Fixed
|
||||
|
||||
- Preserved queued steering and follow-up messages when a continuation is cancelled before or during pre-dequeue hooks, and propagated the caller's cancellation signal through every continuation model-call loop.
|
||||
### Added
|
||||
|
||||
- Added an opt-in `retry` option to `instrumentedCompleteSimple` (`InstrumentedChatSpanOptions.retry`) that re-issues a oneshot completion on transient Anthropic failures. Opt-in rather than default-on because `oneshotKind` is free-form and callers may pass arbitrary `ctx.tools`, so the funnel cannot itself prove a request is replay-safe. Response headers are captured per attempt and cleared between attempts, so a stale `retry-after` can never be reused for a later failure.
|
||||
|
||||
### Fixed
|
||||
|
||||
- Fixed `/handoff`, branch summarization and manual `/compact` aborting on the first transient provider blip. Each is a single, side-effect-free completion whose result is parsed after it resolves, so an Anthropic `overloaded_error` / 429 / 529 now retries instead of failing the whole operation — previously one blip left the user's context full.
|
||||
- Added `SummaryOptions.oneshotRetry` so the compaction summarization oneshots can be retried by the caller that needs it without inflating the caller that does not. Manual `/compact` has no outer loop and gets retry by default; auto-compaction passes `false` because `session-maintenance` already retries the whole attempt, and nesting would multiply the request budget (10 outer x 3 inner) while stacking each outer wait on an inner backoff.
|
||||
|
||||
## [17.2.6] - 2026-08-03
|
||||
|
||||
|
||||
@@ -338,7 +338,7 @@ export async function generateBranchSummary(
|
||||
model,
|
||||
{ systemPrompt: [SUMMARIZATION_SYSTEM_PROMPT], messages: summarizationMessages },
|
||||
{ apiKey, signal, maxTokens: 2048, metadata },
|
||||
{ telemetry: options.telemetry, oneshotKind: "branch_summary", completeImpl: options.completeImpl },
|
||||
{ telemetry: options.telemetry, oneshotKind: "branch_summary", completeImpl: options.completeImpl, retry: {} },
|
||||
);
|
||||
|
||||
// Check if aborted or errored
|
||||
|
||||
@@ -16,6 +16,7 @@ import {
|
||||
type Message,
|
||||
type MessageAttribution,
|
||||
type Model,
|
||||
type OneshotRetryOptions,
|
||||
type ProviderSessionState,
|
||||
type SimpleStreamOptions,
|
||||
type Tool,
|
||||
@@ -831,6 +832,31 @@ export interface SummaryOptions {
|
||||
ctx: Context,
|
||||
options: SimpleStreamOptions,
|
||||
) => Promise<AssistantMessage>;
|
||||
/**
|
||||
* Transient-failure retry for the summarization oneshots (`generateSummary`,
|
||||
* `generateShortSummary`, `generateTurnPrefixSummary`).
|
||||
*
|
||||
* Defaults to enabled, which is what a one-shot caller such as manual
|
||||
* `/compact` needs: a single Anthropic `overloaded_error` / 429 / 529 should
|
||||
* not abort compaction and leave the context full.
|
||||
*
|
||||
* Pass `false` when the CALLER already owns a retry loop around the whole
|
||||
* compaction attempt — auto-compaction does — otherwise the two budgets
|
||||
* multiply (10 outer attempts x 3 inner = 30 requests) and each outer wait
|
||||
* stacks on top of the inner backoff.
|
||||
*/
|
||||
oneshotRetry?: OneshotRetryOptions | false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolve the oneshot retry policy for a summarization call. Enabled by default
|
||||
* so a lone transient blip cannot abort compaction; `false` opts out for callers
|
||||
* that already retry the whole attempt (see `SummaryOptions.oneshotRetry`).
|
||||
*/
|
||||
function summaryOneshotRetry(options: SummaryOptions | undefined): OneshotRetryOptions | undefined {
|
||||
const configured = options?.oneshotRetry;
|
||||
if (configured === false) return undefined;
|
||||
return configured ?? {};
|
||||
}
|
||||
|
||||
function localCodexCompaction(options: SummaryOptions | undefined) {
|
||||
@@ -935,7 +961,12 @@ export async function generateSummary(
|
||||
providerSessionState: options?.providerSessionState,
|
||||
codexCompaction: localCodexCompaction(options),
|
||||
},
|
||||
{ telemetry: options?.telemetry, oneshotKind: "compaction_summary", completeImpl: options?.completeImpl },
|
||||
{
|
||||
telemetry: options?.telemetry,
|
||||
oneshotKind: "compaction_summary",
|
||||
completeImpl: options?.completeImpl,
|
||||
retry: summaryOneshotRetry(options),
|
||||
},
|
||||
);
|
||||
|
||||
if (response.stopReason === "error") {
|
||||
@@ -1034,13 +1065,14 @@ export async function generateHandoffFromContext(
|
||||
telemetry: options.telemetry,
|
||||
oneshotKind: "handoff",
|
||||
completeImpl: options.completeImpl,
|
||||
retry: {},
|
||||
});
|
||||
if (response.stopReason === "error" && shouldRetryHandoffWithAutoToolChoice(response)) {
|
||||
response = await instrumentedCompleteSimple(
|
||||
model,
|
||||
context,
|
||||
{ ...requestOptions, toolChoice: "auto" },
|
||||
{ telemetry: options.telemetry, oneshotKind: "handoff", completeImpl: options.completeImpl },
|
||||
{ telemetry: options.telemetry, oneshotKind: "handoff", completeImpl: options.completeImpl, retry: {} },
|
||||
);
|
||||
}
|
||||
|
||||
@@ -1143,7 +1175,12 @@ async function generateShortSummary(
|
||||
providerSessionState: options?.providerSessionState,
|
||||
codexCompaction: localCodexCompaction(options),
|
||||
},
|
||||
{ telemetry: options?.telemetry, oneshotKind: "compaction_short_summary", completeImpl: options?.completeImpl },
|
||||
{
|
||||
telemetry: options?.telemetry,
|
||||
oneshotKind: "compaction_short_summary",
|
||||
completeImpl: options?.completeImpl,
|
||||
retry: summaryOneshotRetry(options),
|
||||
},
|
||||
);
|
||||
|
||||
if (response.stopReason === "error") {
|
||||
@@ -1719,7 +1756,12 @@ async function generateTurnPrefixSummary(
|
||||
providerSessionState: options?.providerSessionState,
|
||||
codexCompaction: localCodexCompaction(options),
|
||||
},
|
||||
{ telemetry: options?.telemetry, oneshotKind: "compaction_turn_prefix", completeImpl: options?.completeImpl },
|
||||
{
|
||||
telemetry: options?.telemetry,
|
||||
oneshotKind: "compaction_turn_prefix",
|
||||
completeImpl: options?.completeImpl,
|
||||
retry: summaryOneshotRetry(options),
|
||||
},
|
||||
);
|
||||
|
||||
if (response.stopReason === "error") {
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
import { describe, expect, it } from "bun:test";
|
||||
import { generateSummary } from "@oh-my-pi/pi-agent-core/compaction";
|
||||
import type { AgentMessage } from "@oh-my-pi/pi-agent-core/types";
|
||||
import type { AssistantMessage, Model, Usage } from "@oh-my-pi/pi-ai/types";
|
||||
|
||||
/**
|
||||
* Defends `SummaryOptions.oneshotRetry`, the split that lets manual `/compact`
|
||||
* survive a transient provider blip without inflating auto-compaction's budget.
|
||||
*
|
||||
* Both paths call the same `generateSummary`, so the policy cannot be a constant
|
||||
* inside it. Auto-compaction wraps the whole attempt in its own retry loop
|
||||
* (`session-maintenance.ts`), and a nested inner loop would multiply requests
|
||||
* (10 outer x 3 inner) while stacking each outer wait on an inner backoff.
|
||||
* Manual `/compact` has no outer loop: without retry, one `overloaded_error`
|
||||
* aborts compaction and leaves the user's context full.
|
||||
*
|
||||
* A transient failure arrives as a **resolved** `AssistantMessage` with
|
||||
* `stopReason: "error"`, which is why `completeImpl` returning that value —
|
||||
* rather than throwing — is the realistic stub here.
|
||||
*/
|
||||
|
||||
const emptyUsage = (): Usage =>
|
||||
({
|
||||
input: 0,
|
||||
output: 0,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
}) as unknown as Usage;
|
||||
|
||||
const model = {
|
||||
id: "claude-sonnet-4-6",
|
||||
provider: "anthropic",
|
||||
api: "anthropic-messages",
|
||||
baseUrl: "https://api.anthropic.com",
|
||||
maxTokens: 8192,
|
||||
} as unknown as Model;
|
||||
|
||||
// `ApiKey` is `string | ApiKeyResolver`; the stubbed `completeImpl` never uses it.
|
||||
const apiKey = "test-key";
|
||||
|
||||
const messages = [{ role: "user", content: "summarize this session", timestamp: 0 }] as unknown as AgentMessage[];
|
||||
|
||||
function overloaded(): AssistantMessage {
|
||||
return {
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "" }],
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-6",
|
||||
usage: emptyUsage(),
|
||||
stopReason: "error",
|
||||
errorMessage: "overloaded_error: Overloaded",
|
||||
errorStatus: 529,
|
||||
timestamp: 0,
|
||||
} as unknown as AssistantMessage;
|
||||
}
|
||||
|
||||
function summary(text: string): AssistantMessage {
|
||||
return {
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text }],
|
||||
provider: "anthropic",
|
||||
model: "claude-sonnet-4-6",
|
||||
usage: emptyUsage(),
|
||||
stopReason: "stop",
|
||||
timestamp: 0,
|
||||
} as unknown as AssistantMessage;
|
||||
}
|
||||
|
||||
describe("SummaryOptions.oneshotRetry", () => {
|
||||
it("retries a transient failure by default, so manual /compact survives a blip", async () => {
|
||||
let calls = 0;
|
||||
const text = await generateSummary(messages, model, 10_000, apiKey, undefined, undefined, undefined, {
|
||||
// No `oneshotRetry`: the manual `/compact` shape. The one real backoff
|
||||
// wait (default 500ms) is the price of asserting the DEFAULT rather than
|
||||
// a value this test picked for itself.
|
||||
completeImpl: () => {
|
||||
calls += 1;
|
||||
return Promise.resolve(calls === 1 ? overloaded() : summary("recovered summary"));
|
||||
},
|
||||
});
|
||||
|
||||
expect(calls).toBe(2);
|
||||
expect(text).toContain("recovered summary");
|
||||
});
|
||||
|
||||
it("makes exactly one attempt when the caller owns the retry loop", async () => {
|
||||
let calls = 0;
|
||||
const attempt = generateSummary(messages, model, 10_000, apiKey, undefined, undefined, undefined, {
|
||||
// What auto-compaction passes: its own loop re-runs the whole attempt.
|
||||
oneshotRetry: false,
|
||||
completeImpl: () => {
|
||||
calls += 1;
|
||||
return Promise.resolve(overloaded());
|
||||
},
|
||||
});
|
||||
|
||||
// The failure must surface for the outer loop to classify and retry.
|
||||
await expect(attempt).rejects.toThrow();
|
||||
expect(calls).toBe(1);
|
||||
});
|
||||
});
|
||||
@@ -2625,6 +2625,11 @@ export class SessionMaintenance {
|
||||
providerSessionState: this.#host.providerSessionState,
|
||||
preferWebsockets: this.#host.preferWebsockets,
|
||||
codexCompaction,
|
||||
// This loop already retries the whole compaction attempt on
|
||||
// transient errors, so the summarization oneshots must not
|
||||
// retry too — the budgets would multiply and each outer
|
||||
// wait would stack on top of an inner backoff.
|
||||
oneshotRetry: false,
|
||||
},
|
||||
);
|
||||
break;
|
||||
|
||||
Reference in New Issue
Block a user