From aa463f90089d3ebca516d44e25e941eca6330b2a Mon Sep 17 00:00:00 2001 From: roboomp Date: Sat, 20 Jun 2026 07:43:03 +0000 Subject: [PATCH] fix(compaction): aligned /compact remote readiness with candidate selection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Calling /compact remote with an OpenAI/Responses active model + a non-remote compactionModel (e.g. an Anthropic summarizer) used to leave remoteReady=true on the readiness check via shouldUseOpenAiRemoteCompaction( this.model), but #compactWithFallbackModel walked the candidate chain starting from the configured compactionModel and ran a local summary on it — silently doing the opposite of the explicit mode. Make the readiness check and candidate selection share one source of truth. When /compact remote is requested and no compaction.remoteEndpoint is set (an endpoint short-circuits per-model gating in compact()), filter the candidate chain through shouldUseOpenAiRemoteCompaction so non-remote fallbacks are skipped. If the filter empties the chain, warn and fall back to the unfiltered chain so the operation still completes — matching the spirit of the prior warning. The filter is threaded through #getCompactionModelCandidates / #resolveCompactionModelCandidates and the resolved candidates are passed into #compactWithFallbackModel so both paths see the same list. Added a regression test that wires an OpenAI active model with an Anthropic compactionModel, invokes session.compact({ mode: 'remote' }), and asserts the OpenAI model — not the configured compactionModel — is the first candidate handed to compact(). Fixes #3104 --- .../coding-agent/src/session/agent-session.ts | 52 ++++++++------ .../compaction-prefer-current-model.test.ts | 68 +++++++++++++++++++ 2 files changed, 101 insertions(+), 19 deletions(-) diff --git a/packages/coding-agent/src/session/agent-session.ts b/packages/coding-agent/src/session/agent-session.ts index 7b023256b..d2b946939 100644 --- a/packages/coding-agent/src/session/agent-session.ts +++ b/packages/coding-agent/src/session/agent-session.ts @@ -7610,22 +7610,25 @@ export class AgentSession { const effectiveSettings = compactMode ? { ...compactionSettings, ...compactMode.overrides } : compactionSettings; - if (compactMode?.requiresRemote) { - const compactionTarget = this.#resolveCompactionConfiguredTarget( - this.model, - this.#modelRegistry.getAvailable(), + // /compact remote demands provider-native compaction. When no remote + // endpoint is configured (one would override per-model gating in + // compact()), drop fallback candidates that aren't remote-capable so the + // engine never silently runs a local summary on a configured-but-non- + // remote compactionModel. If filtering empties the chain, warn and fall + // back to the full chain so the operation still completes. + const availableModels = this.#modelRegistry.getAvailable(); + const requireProviderRemote = Boolean(compactMode?.requiresRemote && !effectiveSettings.remoteEndpoint); + let compactionCandidates = this.#getCompactionModelCandidates( + availableModels, + requireProviderRemote ? shouldUseOpenAiRemoteCompaction : undefined, + ); + if (requireProviderRemote && compactionCandidates.length === 0) { + this.emitNotice( + "warning", + `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", ); - const remoteReady = - Boolean(effectiveSettings.remoteEndpoint) || - shouldUseOpenAiRemoteCompaction(this.model) || - (compactionTarget ? shouldUseOpenAiRemoteCompaction(compactionTarget) : false); - if (!remoteReady) { - this.emitNotice( - "warning", - `remote compaction is unavailable for ${this.model.id} (no remote endpoint configured) — using a local summary instead`, - "compaction", - ); - } + compactionCandidates = this.#getCompactionModelCandidates(availableModels); } const pathEntries = this.sessionManager.getBranch(); const preparation = prepareCompaction(pathEntries, effectiveSettings); @@ -7759,6 +7762,7 @@ export class AgentSession { remoteInstructions: this.#obfuscateForProvider(this.#baseSystemPrompt.join("\n\n")), convertToLlm: messages => this.#convertToLlmForSideRequest(messages), }, + compactionCandidates, ); summary = result.summary; shortSummary = result.shortSummary; @@ -9104,11 +9108,15 @@ export class AgentSession { }); } - #getCompactionModelCandidates(availableModels: Model[]): Model[] { - return this.#resolveCompactionModelCandidates(this.model, availableModels); + #getCompactionModelCandidates(availableModels: Model[], filter?: (model: Model) => boolean): Model[] { + return this.#resolveCompactionModelCandidates(this.model, availableModels, filter); } - #resolveCompactionModelCandidates(preferredModel: Model | null | undefined, availableModels: Model[]): Model[] { + #resolveCompactionModelCandidates( + preferredModel: Model | null | undefined, + availableModels: Model[], + filter?: (model: Model) => boolean, + ): Model[] { const candidates: Model[] = []; const seen = new Set(); @@ -9117,6 +9125,10 @@ export class AgentSession { const key = this.#getModelKey(model); 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. + if (filter && !filter(model)) return; candidates.push(model); }; @@ -9169,8 +9181,10 @@ export class AgentSession { customInstructions: string | undefined, signal: AbortSignal, options?: SummaryOptions, + precomputedCandidates?: Model[], ): Promise { - const candidates = this.#getCompactionModelCandidates(this.#modelRegistry.getAvailable()); + const candidates = + precomputedCandidates ?? this.#getCompactionModelCandidates(this.#modelRegistry.getAvailable()); const telemetry = resolveTelemetry(this.agent.telemetry, this.sessionId); for (const candidate of candidates) { diff --git a/packages/coding-agent/test/compaction-prefer-current-model.test.ts b/packages/coding-agent/test/compaction-prefer-current-model.test.ts index 4d2c45ec8..cdbfbc802 100644 --- a/packages/coding-agent/test/compaction-prefer-current-model.test.ts +++ b/packages/coding-agent/test/compaction-prefer-current-model.test.ts @@ -162,4 +162,72 @@ describe("compaction prefers the current session model over modelRoles.default", ); expect(`${session.model?.provider}/${session.model?.id}`).toBe(`${currentModel.provider}/${currentModel.id}`); }); + + it("/compact remote skips a non-remote-capable compactionModel and uses the active remote-capable model", async () => { + // Active model is OpenAI (provider-native remote-capable per + // shouldUseOpenAiRemoteCompaction). compactionModel points at an + // Anthropic model that is NOT remote-capable, so the default candidate + // chain would try Anthropic first and run a local summary — exactly the + // silent-fallback the reviewer flagged for `/compact remote`. The fix + // filters non-remote candidates in this mode, so the spy must observe + // the OpenAI model as the first invocation. + const baseCurrentModel = getBundledModel("openai", "gpt-5"); + const nonRemoteCompactionModel = getBundledModel("anthropic", "claude-sonnet-4-5"); + if (!baseCurrentModel || !nonRemoteCompactionModel) { + throw new Error("Expected bundled test models to exist"); + } + const currentModel = buildModel({ + ...baseCurrentModel, + compactionModel: `${nonRemoteCompactionModel.provider}/${nonRemoteCompactionModel.id}`, + compat: baseCurrentModel.compatConfig, + }); + + const agent = new Agent({ + initialState: { + model: currentModel, + systemPrompt: ["Test"], + tools: [], + messages: [], + }, + }); + + authStorage = await AuthStorage.create(path.join(tempDir.path(), "testauth.db")); + authStorage.setRuntimeApiKey(currentModel.provider, "openai-token"); + authStorage.setRuntimeApiKey(nonRemoteCompactionModel.provider, "anthropic-token"); + modelRegistry = new ModelRegistry(authStorage, path.join(tempDir.path(), "models.yml")); + + session = new AgentSession({ + agent, + sessionManager: SessionManager.inMemory(), + settings: Settings.isolated({ "compaction.keepRecentTokens": 1 }), + modelRegistry, + }); + session.subscribe(() => {}); + + for (const [userText, assistantText] of [ + ["first question", "first answer"], + ["second question", "second answer"], + ] as const) { + const user = userMsg(userText); + const assistant = assistantMsg(assistantText); + session.agent.appendMessage(user); + session.sessionManager.appendMessage(user); + session.agent.appendMessage(assistant); + session.sessionManager.appendMessage(assistant); + } + + const compactSpy = vi.spyOn(compactionModule, "compact").mockImplementation(async (preparation, model) => ({ + summary: "ok", + shortSummary: "ok short", + firstKeptEntryId: preparation.firstKeptEntryId, + tokensBefore: 1, + details: { provider: model.provider }, + })); + + await session.compact(undefined, { mode: "remote" }); + + expect(compactSpy).toHaveBeenCalled(); + const [, firstCandidate] = compactSpy.mock.calls[0]!; + expect(`${firstCandidate.provider}/${firstCandidate.id}`).toBe(`${currentModel.provider}/${currentModel.id}`); + }); });