diff --git a/packages/agent/CHANGELOG.md b/packages/agent/CHANGELOG.md index b4487a72a..6a8e07bed 100644 --- a/packages/agent/CHANGELOG.md +++ b/packages/agent/CHANGELOG.md @@ -23,6 +23,9 @@ ### Fixed - Fixed proxy-stream clients dropping finalized provider-only content blocks, including Anthropic native web-search history, by allowing `done` and `error` events to carry terminal assistant content while retaining delta-reconstructed content from older proxy servers that omit it ([#6703](https://github.com/can1357/oh-my-pi/issues/6703)). +### Fixed + +- Provider-native compaction failures now surface their transport error instead of silently switching to generic summarization; streaming V2 still falls back to native V1 when available. ## [17.1.4] - 2026-07-26 diff --git a/packages/agent/src/compaction/compaction.ts b/packages/agent/src/compaction/compaction.ts index cb1317b7e..0563cd046 100644 --- a/packages/agent/src/compaction/compaction.ts +++ b/packages/agent/src/compaction/compaction.ts @@ -1406,6 +1406,7 @@ export async function compact( ...recentMessages, ]; let usedRemoteCompaction = false; + let nativeCompactionError: unknown; if ( settings.remoteEnabled !== false && settings.remoteStreamingV2Enabled !== false && @@ -1467,7 +1468,8 @@ export async function compact( // swallowing it here would downgrade Esc into "fall back to local // summarization" and keep compaction running on an aborted signal. if (signal?.aborted) throw err; - logger.warn("OpenAI V2 remote compaction failed, falling back to V1/local summarization", { + nativeCompactionError = err; + logger.warn("OpenAI V2 remote compaction failed, falling back to V1 remote compaction", { error: err instanceof Error ? err.message : String(err), model: model.id, provider: model.provider, @@ -1517,7 +1519,8 @@ export async function compact( // swallowing it here would downgrade Esc into "fall back to local // summarization" and keep compaction running on an aborted signal. if (signal?.aborted) throw err; - logger.warn("OpenAI remote compaction failed, falling back to local summarization", { + nativeCompactionError = err; + logger.warn("OpenAI remote compaction failed", { error: err instanceof Error ? err.message : String(err), model: model.id, provider: model.provider, @@ -1526,6 +1529,10 @@ export async function compact( } } + if (!usedRemoteCompaction && nativeCompactionError !== undefined) { + throw nativeCompactionError; + } + // Generate summaries (can be parallel if both needed) and merge into one let summary: string; diff --git a/packages/agent/test/remote-compaction.test.ts b/packages/agent/test/remote-compaction.test.ts index 155080b75..18d4e48f9 100644 --- a/packages/agent/test/remote-compaction.test.ts +++ b/packages/agent/test/remote-compaction.test.ts @@ -1687,6 +1687,31 @@ describe("compact() remote compaction failure handling", () => { expect(JSON.stringify(sameProviderActive?.messagesToSummarize ?? [])).not.toContain("ORIGINAL ALPHA port 4242"); }); + test("V2 native failure falls back to V1 without generic summarization", async () => { + const completeSpy = vi.spyOn(ai, "completeSimple").mockResolvedValue(localSummaryMessage("local summary")); + const preparation = makePreparation(); + preparation.settings = { ...preparation.settings, remoteStreamingV2Enabled: true }; + const model = makeOpenAiModel({ + remoteCompaction: { enabled: true, v2StreamingEnabled: true }, + }); + const requestedUrls: string[] = []; + const fetchMock: FetchImpl = async input => { + const url = String(input); + requestedUrls.push(url); + if (url.endsWith("/responses/compact")) { + return Response.json({ output: [{ type: "compaction", encrypted_content: "enc-v1" }] }); + } + return new Response("V2 unavailable", { status: 502, statusText: "Bad Gateway" }); + }; + + const result = await compact(preparation, model, "test-key", undefined, undefined, { fetch: fetchMock }); + + expect(requestedUrls.some(url => url.endsWith("/responses"))).toBe(true); + expect(requestedUrls.some(url => url.endsWith("/responses/compact"))).toBe(true); + expect(result.shortSummary).toBe("Remote compaction"); + expect(completeSpy).not.toHaveBeenCalled(); + }); + test("user abort during the remote compact request rejects without falling back to local summarization", async () => { // Contract: Esc is a cancellation, not a remote failure. Before the fix // the AbortError was swallowed by the fallback catch and compaction kept @@ -1760,16 +1785,16 @@ describe("compact() remote compaction failure handling", () => { }); }); - test("remote compact server failure without abort still falls back to local summarization", async () => { + test("native compaction server failure rejects without generic summarization", async () => { const completeSpy = vi.spyOn(ai, "completeSimple").mockResolvedValue(localSummaryMessage("local summary")); const fetchMock: FetchImpl = async () => new Response("nope", { status: 500, statusText: "Internal Server Error" }); - const result = await compact(makePreparation(), makeOpenAiModel(), "test-key", undefined, undefined, { - fetch: fetchMock, - }); - - expect(result.summary).toContain("local summary"); - expect(completeSpy).toHaveBeenCalled(); + await expect( + compact(makePreparation(), makeOpenAiModel(), "test-key", undefined, undefined, { + fetch: fetchMock, + }), + ).rejects.toThrow("Remote compaction failed"); + expect(completeSpy).not.toHaveBeenCalled(); }); }); diff --git a/packages/coding-agent/CHANGELOG.md b/packages/coding-agent/CHANGELOG.md index 6859f52d6..6350cfa33 100644 --- a/packages/coding-agent/CHANGELOG.md +++ b/packages/coding-agent/CHANGELOG.md @@ -161,6 +161,9 @@ - Fixed MiMo models using hashline edit mode by default despite needing the same replace-mode fallback as Kimi. ([#3772](https://github.com/can1357/oh-my-pi/issues/3772)) - Fixed `omp` refusing to start on Windows when no `bash.exe` is discoverable — most visibly with scoop-installed Git, whose manifest shims `sh.exe`/`git.exe` but never `bash.exe`, so PATH lookup missed it. Startup threw `No bash shell found` while merely building the bash tool description, even though bash tool commands always execute in the embedded brush-core shell and need no host bash. Shell discovery now also checks `GIT_INSTALL_ROOT`, scoop and per-user Git for Windows install roots, and `sh.exe` on PATH, then falls back to `cmd.exe` for the spawn-only paths (interactive PTY, ACP client terminals) instead of failing; the cmd fallback is never used to wrap user-shell commands — brush runs the POSIX line directly. - Added a selectable voice setting for `/live` realtime sessions ([#6566](https://github.com/can1357/oh-my-pi/issues/6566)). +### Fixed + +- Native compaction now keeps implicit role and largest-context fallbacks on the active provider, preventing a provider-native request from silently becoming another provider's generic summary. Explicit compaction models and soft compaction retain their existing fallback behavior. ## [17.1.4] - 2026-07-26 diff --git a/packages/coding-agent/src/session/session-maintenance.ts b/packages/coding-agent/src/session/session-maintenance.ts index df65f9a32..5222c8508 100644 --- a/packages/coding-agent/src/session/session-maintenance.ts +++ b/packages/coding-agent/src/session/session-maintenance.ts @@ -34,6 +34,7 @@ import { type SummaryOptions, shouldCompact, shouldUseOpenAiRemoteCompaction, + shouldUseCompactionV2Streaming, } from "@oh-my-pi/pi-agent-core/compaction"; import { DEFAULT_PRUNE_CONFIG, @@ -567,6 +568,7 @@ export class SessionMaintenance { let compactionCandidates = this.#getCompactionModelCandidates( availableModels, requireProviderRemote ? shouldUseOpenAiRemoteCompaction : undefined, + effectiveSettings, ); if (requireProviderRemote && compactionCandidates.length === 0) { this.#host.emitNotice( @@ -574,7 +576,7 @@ export class SessionMaintenance { `remote compaction is unavailable for ${this.#model.id} (no remote endpoint configured and no provider-native remote-capable model in the fallback chain) — using a local summary instead`, "compaction", ); - compactionCandidates = this.#getCompactionModelCandidates(availableModels); + compactionCandidates = this.#getCompactionModelCandidates(availableModels, undefined, effectiveSettings); } const pathEntries = this.#host.sessionManager.getBranch(); const preparation = prepareCompaction(pathEntries, effectiveSettings, this.#model); @@ -1390,46 +1392,73 @@ export class SessionMaintenance { return candidate; } - #getCompactionModelCandidates(availableModels: Model[], filter?: (model: Model) => boolean): Model[] { - return this.resolveCompactionModelCandidates(this.#model, availableModels, filter); + #getCompactionModelCandidates( + availableModels: Model[], + filter: ((model: Model) => boolean) | undefined, + settings: Pick, + ): Model[] { + return this.resolveCompactionModelCandidates( + this.#model, + availableModels, + filter, + settings.remoteEnabled !== false, + settings.remoteStreamingV2Enabled !== false, + ); } resolveCompactionModelCandidates( preferredModel: Model | null | undefined, availableModels: Model[], filter?: (model: Model) => boolean, + remoteEnabled = this.#host.settings.getGroup("compaction").remoteEnabled !== false, + remoteStreamingV2Enabled = + this.#host.settings.getGroup("compaction").remoteStreamingV2Enabled !== false, ): Model[] { const candidates: Model[] = []; const seen = new Set(); + const hasEffectiveNativeCompaction = (model: Model): boolean => + remoteEnabled && + (shouldUseOpenAiRemoteCompaction(model) || + (remoteStreamingV2Enabled && shouldUseCompactionV2Streaming(model))); + const nativeProvider = + preferredModel && hasEffectiveNativeCompaction(preferredModel) ? preferredModel.provider : undefined; - const addCandidate = (model: Model | undefined): void => { + const addCandidate = (model: Model | undefined, source: "explicit" | "current" | "implicit"): void => { if (!model) return; const key = `${model.provider}/${model.id}`; if (seen.has(key)) return; seen.add(key); - // `seen` still tracks rejected models so the largest-context fallback - // scan below doesn't reintroduce them; the filter just suppresses - // inclusion in this caller's candidate chain. + // Explicit targets and the active model retain their established + // semantics. Implicit role/context fallbacks must not turn a native + // compaction request into a different provider's generic summary. + if ( + source === "implicit" && + nativeProvider !== undefined && + (model.provider !== nativeProvider || !hasEffectiveNativeCompaction(model)) + ) { + return; + } if (filter && !filter(model)) return; candidates.push(model); }; if (preferredModel) { - addCandidate(resolveCompactionConfiguredTarget(preferredModel, availableModels)); + addCandidate(resolveCompactionConfiguredTarget(preferredModel, availableModels), "explicit"); } - addCandidate(preferredModel ?? undefined); + addCandidate(preferredModel ?? undefined, "current"); for (const role of MODEL_ROLE_IDS) { addCandidate( resolveRoleModelFull(this.#host.settings, role, availableModels, preferredModel ?? undefined).model, + "implicit", ); } const sortedByContext = [...availableModels].sort((a, b) => (b.contextWindow ?? 0) - (a.contextWindow ?? 0)); for (const model of sortedByContext) { - if (!seen.has(`${model.provider}/${model.id}`)) { - addCandidate(model); - break; - } + if (seen.has(`${model.provider}/${model.id}`)) continue; + const candidateCount = candidates.length; + addCandidate(model, "implicit"); + if (candidates.length > candidateCount) break; } return candidates; @@ -1456,7 +1485,8 @@ export class SessionMaintenance { precomputedCandidates?: Model[], ): Promise { const candidates = - precomputedCandidates ?? this.#getCompactionModelCandidates(this.#host.modelRegistry.getAvailable()); + precomputedCandidates ?? + this.#getCompactionModelCandidates(this.#host.modelRegistry.getAvailable(), undefined, preparation.settings); const telemetry = resolveTelemetry(this.#host.agent.telemetry, this.#host.sessionId()); for (const candidate of candidates) { @@ -2472,7 +2502,7 @@ export class SessionMaintenance { details = snapcompactResult.details; preserveData = { ...(compactionPrep.preserveData ?? {}), ...(snapcompactResult.preserveData ?? {}) }; } else { - const candidates = this.#getCompactionModelCandidates(availableModels); + const candidates = this.#getCompactionModelCandidates(availableModels, undefined, compactionSettings); const retrySettings = this.#host.settings.getGroup("retry"); const telemetry = resolveTelemetry(this.#host.agent.telemetry, this.#host.sessionId()); let compactResult: CompactionResult | undefined; diff --git a/packages/coding-agent/test/issue-986-compaction-auth-fallback.test.ts b/packages/coding-agent/test/issue-986-compaction-auth-fallback.test.ts index e49551a73..2022a1c19 100644 --- a/packages/coding-agent/test/issue-986-compaction-auth-fallback.test.ts +++ b/packages/coding-agent/test/issue-986-compaction-auth-fallback.test.ts @@ -37,7 +37,13 @@ describe("issue #986 compaction auth fallback", () => { throw new Error("Expected bundled test models to exist"); } - const settings = Settings.isolated({ "compaction.keepRecentTokens": 1, "compaction.strategy": "context-full" }); + const settings = Settings.isolated({ + "compaction.keepRecentTokens": 1, + "compaction.strategy": "context-full", + // This suite covers the portable summarizer's auth fallback. Native + // compaction keeps its implicit candidate chain provider-isolated. + "compaction.remoteEnabled": false, + }); if (options?.fallbackModelRole) { settings.setModelRole(options.fallbackModelRole, `${fallbackModel.provider}/${fallbackModel.id}`); } diff --git a/packages/coding-agent/test/native-compaction-provider-isolation.test.ts b/packages/coding-agent/test/native-compaction-provider-isolation.test.ts new file mode 100644 index 000000000..e6dd2dda1 --- /dev/null +++ b/packages/coding-agent/test/native-compaction-provider-isolation.test.ts @@ -0,0 +1,97 @@ +import { describe, expect, it } from "bun:test"; +import type { Model } from "@oh-my-pi/pi-ai"; +import { buildModel } from "@oh-my-pi/pi-catalog/build"; +import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings"; +import { + SessionMaintenance, + type SessionMaintenanceHost, +} from "@oh-my-pi/pi-coding-agent/session/session-maintenance"; + +function model( + id: string, + provider: string, + contextWindow: number, + remoteCompaction?: Model["remoteCompaction"], +): Model { + return buildModel({ + id, + name: id, + api: "openai-responses", + provider, + baseUrl: "https://example.test/v1", + reasoning: false, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow, + maxTokens: 4096, + remoteCompaction, + }); +} + +function maintenance(settings: Settings): SessionMaintenance { + return new SessionMaintenance({ settings } as SessionMaintenanceHost); +} + +describe("native compaction provider isolation", () => { + it("keeps implicit role and context fallbacks on the current native provider", () => { + const settings = Settings.isolated(); + const current = model("native-current", "native-provider", 100_000, { enabled: true }); + const genericRole = model("generic-role", "generic-provider", 90_000); + const nativeFallback = model("native-fallback", "native-provider", 80_000, { enabled: true }); + settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`); + + const candidates = maintenance(settings).resolveCompactionModelCandidates( + current, + [current, genericRole, nativeFallback], + undefined, + true, + true, + ); + + expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([ + "native-provider/native-current", + "native-provider/native-fallback", + ]); + }); + + it("retains generic implicit fallbacks when provider-native compaction is disabled", () => { + const settings = Settings.isolated(); + const current = model("native-current", "native-provider", 100_000, { enabled: true }); + const genericRole = model("generic-role", "generic-provider", 90_000); + settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`); + + const candidates = maintenance(settings).resolveCompactionModelCandidates( + current, + [current, genericRole], + undefined, + false, + true, + ); + + expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([ + "native-provider/native-current", + "generic-provider/generic-role", + ]); + }); + + it("recognizes V2-only native capability under the effective streaming setting", () => { + const settings = Settings.isolated(); + const current = model("v2-current", "v2-provider", 100_000, { v2StreamingEnabled: true }); + const genericRole = model("generic-role", "generic-provider", 90_000); + const nativeFallback = model("v2-fallback", "v2-provider", 80_000, { v2StreamingEnabled: true }); + settings.setModelRole("smol", `${genericRole.provider}/${genericRole.id}`); + + const candidates = maintenance(settings).resolveCompactionModelCandidates( + current, + [current, genericRole, nativeFallback], + undefined, + true, + true, + ); + + expect(candidates.map(candidate => `${candidate.provider}/${candidate.id}`)).toEqual([ + "v2-provider/v2-current", + "v2-provider/v2-fallback", + ]); + }); +});