fix(compaction): aligned /compact remote readiness with candidate selection
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
This commit is contained in:
@@ -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<string>();
|
||||
|
||||
@@ -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<CompactionResult> {
|
||||
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) {
|
||||
|
||||
@@ -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}`);
|
||||
});
|
||||
});
|
||||
|
||||
Reference in New Issue
Block a user