Merge PR #5440: fix(session): fall back to LLM compaction on manual /compact for text-only models (@roboomp)
This commit is contained in:
can1357
@@ -345,6 +345,19 @@
|
||||
- Fixed subagent yield tool calls being discarded when a soft request budget aborts the assistant turn before the yield event completes.
|
||||
- Fixed --tools filtering in interactive sessions incorrectly disabling deferred MCP tools from configured servers.
|
||||
- Fixed kept-alive task subagents entering infinite provider-call loops after an IRC wake and terminal yield.
|
||||
- Fixed interactive TUI sessions dying with `Unhandled rejection: Cannot set cwd while another same-realm JS runtime is running` after the JS eval worker fell back to the in-process inline path (commonly when the worker could not load `pi_natives`). Concurrent inline eval/browser runtimes now stamp cwd (including the saved `__omp_session__` state) without stealing the exclusive realm, WorkerCore `init` reports failures via `init-failed` instead of throwing out of the microtask path, and constructing a runtime while another same-realm run is live fails explicitly instead of clobbering its globals. ([#4907](https://github.com/can1357/oh-my-pi/pull/4907) by [@cexll](https://github.com/cexll))
|
||||
- Fixed compaction aborting instead of trying an authenticated fallback model when Amazon Bedrock credential resolution fails before a request is sent. ([#5030](https://github.com/can1357/oh-my-pi/pull/5030) by [@usr-bin-roygbiv](https://github.com/usr-bin-roygbiv))
|
||||
- Fixed full-context forks cold-missing OpenAI prompt caches by persisting an inherited provider prompt-cache key separately from the new OMP session id, adding `--prompt-cache-key` for explicit cache affinity, and dropping automatic inheritance when startup changes the model, thinking level, system prompt, or tool schema. ([#5035](https://github.com/can1357/oh-my-pi/issues/5035))
|
||||
- Fixed Codex advisor requests using local `-advisor` session labels as provider session IDs; advisors now use stable UUIDv7 provider identities while keeping labeled transcript names. ([#5040](https://github.com/can1357/oh-my-pi/issues/5040))
|
||||
- Fixed macOS stdio MCP servers launching in a detached session, so `xcrun mcpbridge` can trigger the TCC Apple Events permission prompt and complete startup. ([#4987](https://github.com/can1357/oh-my-pi/issues/4987))
|
||||
- Fixed the ask tool timeout so it auto-selects the recommended option even when the UI selector does not settle on its own. ([#4995](https://github.com/can1357/oh-my-pi/issues/4995))
|
||||
- Fixed LSP workspace diagnostics for Go workspaces so roots with `go.work` are recognized and every `go.work use` module is included in the `go build` package patterns. ([#5038](https://github.com/can1357/oh-my-pi/issues/5038))
|
||||
- Fixed interactive OAuth login success messages waiting on model discovery; `/login xai-oauth` now reports saved credentials immediately while model metadata refreshes in the background. ([#4989](https://github.com/can1357/oh-my-pi/issues/4989))
|
||||
- Fixed Windows bash tool crashes when an explicit timeout fires while a piped command is still streaming output; the JavaScript fallback now reports the timeout without also aborting the native timeout signal. ([#5021](https://github.com/can1357/oh-my-pi/issues/5021))
|
||||
- Fixed subagent `yield` tool calls being discarded when the soft request budget hard-aborted the same assistant turn before the yield result event landed. ([#5006](https://github.com/can1357/oh-my-pi/issues/5006))
|
||||
- Fixed `--tools` filtering in interactive sessions disabling deferred MCP tools; MCP tools discovered from configured servers now stay active when the flag limits only built-in tools. ([#5013](https://github.com/can1357/oh-my-pi/issues/5013))
|
||||
- Fixed kept-alive task subagents entering a repeated provider-call loop after an IRC wake and terminal `yield`. ([#4963](https://github.com/can1357/oh-my-pi/issues/4963))
|
||||
- Fixed manual `/compact` with the snapcompact strategy hard-failing on text-only active models; it now warns and falls back to LLM compaction (mirroring the auto-compaction path) instead of throwing. ([#5064](https://github.com/can1357/oh-my-pi/issues/5064))
|
||||
|
||||
## [16.3.15] - 2026-07-09
|
||||
|
||||
|
||||
@@ -10431,24 +10431,37 @@ export class AgentSession {
|
||||
|
||||
// Strategy honored on manual /compact too. Custom instructions (public
|
||||
// user focus OR internal plan-mode guidance) imply a directed LLM
|
||||
// summary; a text-only model cannot read snapcompact frames. When
|
||||
// snapcompact itself was requested, fail locally instead of silently
|
||||
// converting the "no LLM call" path into a provider-backed summary.
|
||||
// summary; a text-only model cannot read snapcompact frames.
|
||||
const wantsSnapcompact =
|
||||
compactionPrep.kind !== "fromHook" &&
|
||||
effectiveSettings.strategy === "snapcompact" &&
|
||||
!customInstructions &&
|
||||
!options?.internalGuidance;
|
||||
const snapcompactReady = wantsSnapcompact;
|
||||
// `/compact snapcompact` is an explicit no-LLM archive request: honor
|
||||
// its contract by failing locally rather than silently shipping the
|
||||
// transcript to a provider. The default-configured snapcompact
|
||||
// strategy, in contrast, falls back to LLM compaction (mirroring the
|
||||
// auto-compaction path) so a routine /compact still completes on a
|
||||
// text-only model (issue #5064).
|
||||
const explicitSnapcompact = compactMode?.name === "snapcompact";
|
||||
let snapcompactReady = wantsSnapcompact;
|
||||
const snapcompactShapeSetting = this.settings.get("snapcompact.shape");
|
||||
let snapcompactShape: snapcompact.Shape | undefined;
|
||||
if (wantsSnapcompact && !this.model.input.includes("image")) {
|
||||
if (explicitSnapcompact) {
|
||||
this.emitNotice(
|
||||
"warning",
|
||||
`snapcompact needs a vision-capable model (${this.model.id} is text-only)`,
|
||||
"compaction",
|
||||
);
|
||||
throw new Error(`snapcompact cannot run locally: ${this.model.id} is text-only.`);
|
||||
}
|
||||
this.emitNotice(
|
||||
"warning",
|
||||
`snapcompact needs a vision-capable model (${this.model.id} is text-only)`,
|
||||
`snapcompact needs a vision-capable model (${this.model.id} is text-only); falling back to LLM compaction`,
|
||||
"compaction",
|
||||
);
|
||||
throw new Error(`snapcompact cannot run locally: ${this.model.id} is text-only.`);
|
||||
snapcompactReady = false;
|
||||
} else if (snapcompactReady) {
|
||||
const text = snapcompact.serializeConversation(
|
||||
convertToLlm(preparation.messagesToSummarize.concat(preparation.turnPrefixMessages)),
|
||||
|
||||
@@ -0,0 +1,147 @@
|
||||
import { afterEach, 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 { Message, Model } from "@oh-my-pi/pi-ai";
|
||||
import { getBundledModel } from "@oh-my-pi/pi-catalog/models";
|
||||
import { ModelRegistry } from "@oh-my-pi/pi-coding-agent/config/model-registry";
|
||||
import { Settings } from "@oh-my-pi/pi-coding-agent/config/settings";
|
||||
import { AgentSession } from "@oh-my-pi/pi-coding-agent/session/agent-session";
|
||||
import { AuthStorage } from "@oh-my-pi/pi-coding-agent/session/auth-storage";
|
||||
import { SessionManager } from "@oh-my-pi/pi-coding-agent/session/session-manager";
|
||||
import { TempDir } from "@oh-my-pi/pi-utils";
|
||||
|
||||
/**
|
||||
* Regression for issue #5064.
|
||||
*
|
||||
* Manual `/compact` with the default snapcompact strategy hard-threw
|
||||
* ("snapcompact cannot run locally: <id> is text-only") when the active model
|
||||
* lacked image input, even though the auto-compaction path already downgraded
|
||||
* to LLM-backed compaction in the same situation. The manual path MUST mirror
|
||||
* that behavior: warn, then summarize via the LLM fallback candidate chain
|
||||
* (which tries the active text→text model first).
|
||||
*
|
||||
* An *explicit* `/compact snapcompact` (mode override) is a deliberate no-LLM
|
||||
* archive request, so it MUST keep failing locally instead of silently
|
||||
* shipping the transcript to a provider.
|
||||
*/
|
||||
describe("AgentSession manual snapcompact text-only fallback", () => {
|
||||
let session: AgentSession | undefined;
|
||||
let authStorage: AuthStorage | undefined;
|
||||
let tempDir: TempDir | undefined;
|
||||
|
||||
afterEach(async () => {
|
||||
try {
|
||||
await session?.dispose();
|
||||
} finally {
|
||||
authStorage?.close();
|
||||
await tempDir?.remove();
|
||||
vi.restoreAllMocks();
|
||||
session = undefined;
|
||||
authStorage = undefined;
|
||||
tempDir = undefined;
|
||||
}
|
||||
});
|
||||
|
||||
async function createHarness(): Promise<{
|
||||
session: AgentSession;
|
||||
sessionManager: SessionManager;
|
||||
activeModel: Model;
|
||||
notices: string[];
|
||||
}> {
|
||||
const activeModel = getBundledModel("aimlapi", "alibaba/qwen3-coder-480b-a35b-instruct");
|
||||
if (!activeModel) throw new Error("Expected bundled text-only model");
|
||||
expect(activeModel.input).not.toContain("image");
|
||||
|
||||
tempDir = TempDir.createSync("@pi-manual-snapcompact-text-only-");
|
||||
authStorage = await AuthStorage.create(path.join(tempDir.path(), "auth.db"));
|
||||
authStorage.setRuntimeApiKey("aimlapi", "test-key");
|
||||
const modelRegistry = new ModelRegistry(authStorage);
|
||||
|
||||
const agent = new Agent({
|
||||
initialState: { model: activeModel, systemPrompt: ["Test"], tools: [], messages: [] },
|
||||
});
|
||||
const sessionManager = SessionManager.create(tempDir.path(), tempDir.path());
|
||||
const seed: Message[] = [
|
||||
{ role: "user", content: "first question", timestamp: Date.now() },
|
||||
{
|
||||
role: "assistant",
|
||||
content: [{ type: "text", text: "first answer" }],
|
||||
api: activeModel.api,
|
||||
provider: activeModel.provider,
|
||||
model: activeModel.id,
|
||||
stopReason: "stop",
|
||||
usage: {
|
||||
input: 10,
|
||||
output: 10,
|
||||
cacheRead: 0,
|
||||
cacheWrite: 0,
|
||||
totalTokens: 20,
|
||||
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
|
||||
},
|
||||
timestamp: Date.now(),
|
||||
},
|
||||
{ role: "user", content: "second question", timestamp: Date.now() },
|
||||
];
|
||||
for (const message of seed) sessionManager.appendMessage(message);
|
||||
if (!sessionManager.getBranch()[0]?.id) throw new Error("Expected seeded branch entry");
|
||||
|
||||
const settings = Settings.isolated({
|
||||
"compaction.strategy": "snapcompact",
|
||||
"compaction.keepRecentTokens": 1,
|
||||
});
|
||||
session = new AgentSession({ agent, sessionManager, settings, modelRegistry });
|
||||
const notices: string[] = [];
|
||||
session.subscribe(event => {
|
||||
if (event.type === "notice" && event.source === "compaction") notices.push(event.message);
|
||||
});
|
||||
|
||||
return { session, sessionManager, activeModel, notices };
|
||||
}
|
||||
|
||||
it("falls back to LLM compaction instead of throwing on a text-only active model", async () => {
|
||||
const harness = await createHarness();
|
||||
|
||||
const compactSpy = vi.spyOn(compactionModule, "compact").mockImplementation(async (preparation, model) => ({
|
||||
summary: "llm summary",
|
||||
shortSummary: "llm",
|
||||
firstKeptEntryId: preparation.firstKeptEntryId,
|
||||
tokensBefore: 42,
|
||||
details: { provider: model.provider, model: model.id },
|
||||
}));
|
||||
|
||||
const result = await harness.session.compact();
|
||||
|
||||
expect(result.summary).toBe("llm summary");
|
||||
// LLM fallback ran; the active text-only model is tried first.
|
||||
expect(compactSpy).toHaveBeenCalled();
|
||||
const [, firstCandidate] = compactSpy.mock.calls[0]!;
|
||||
expect(`${firstCandidate.provider}/${firstCandidate.id}`).toBe(
|
||||
`${harness.activeModel.provider}/${harness.activeModel.id}`,
|
||||
);
|
||||
expect(harness.notices).toContain(
|
||||
`snapcompact needs a vision-capable model (${harness.activeModel.id} is text-only); falling back to LLM compaction`,
|
||||
);
|
||||
expect(harness.sessionManager.getBranch().find(entry => entry.type === "compaction")).toMatchObject({
|
||||
type: "compaction",
|
||||
summary: "llm summary",
|
||||
});
|
||||
});
|
||||
|
||||
it("still fails locally for explicit /compact snapcompact on a text-only model (no-LLM contract)", async () => {
|
||||
const harness = await createHarness();
|
||||
|
||||
const compactSpy = vi.spyOn(compactionModule, "compact");
|
||||
|
||||
await expect(harness.session.compact(undefined, { mode: "snapcompact" })).rejects.toThrow(
|
||||
`snapcompact cannot run locally: ${harness.activeModel.id} is text-only.`,
|
||||
);
|
||||
|
||||
// Explicit no-LLM request must never reach the provider-backed summarizer.
|
||||
expect(compactSpy).not.toHaveBeenCalled();
|
||||
expect(harness.notices).toContain(
|
||||
`snapcompact needs a vision-capable model (${harness.activeModel.id} is text-only)`,
|
||||
);
|
||||
expect(harness.sessionManager.getBranch().find(entry => entry.type === "compaction")).toBeUndefined();
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user