Skip to content

fix(compaction): enable custom provider remote compaction#3106

Open
roboomp wants to merge 4 commits into
mainfrom
farm/0adff054/custom-provider-compaction
Open

fix(compaction): enable custom provider remote compaction#3106
roboomp wants to merge 4 commits into
mainfrom
farm/0adff054/custom-provider-compaction

Conversation

@roboomp

@roboomp roboomp commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator

Repro

A custom openai-responses model with provider id cliproxy-codex could not enter the native remote compaction path: bun -e 'import { shouldUseOpenAiRemoteCompaction } from "@oh-my-pi/pi-agent-core/compaction/openai"; import { buildModel } from "@oh-my-pi/pi-catalog/build"; const model = buildModel({ id: "gpt-5.5", name: "gpt-5.5", api: "openai-responses", provider: "cliproxy-codex", baseUrl: "http://127.0.0.1:8317/v1", reasoning: true, input: ["text"], cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, contextWindow: 272000, maxTokens: 128000 }); const allowed = shouldUseOpenAiRemoteCompaction(model); console.log(remote compaction allowed: ${allowed}); if (!allowed) process.exit(1);' printed remote compaction allowed: false.

Cause

packages/agent/src/compaction/openai.ts gated OpenAI Responses native compaction on model.provider === "openai" || model.provider === "openai-codex"; packages/coding-agent/src/config/models-config-schema.ts and model-registry.ts had no provider/model metadata to let custom providers declare a compatible compaction adapter, endpoint, or compaction-only model.

Fix

  • Added remoteCompaction metadata to catalog Model/ModelSpec and propagated it through models.yml provider, model, modelOverride, discovery, and runtime provider paths.
  • Taught OpenAI remote compaction to honor explicit remoteCompaction.enabled, api, endpoint, and request model for custom providers while preserving built-in OpenAI/OpenAI Codex defaults.
  • Added compactionModel as a summary-only model selector and candidate priority so compaction can run on a separate model without changing the active session model.
  • Covered native custom-provider compaction, config propagation, and active-model preservation with regression tests.

Verification

Reproduced the original gate returning remote compaction allowed: false; after the fix, the same custom provider with remoteCompaction.enabled: true returns remote compaction allowed: true. Ran bun test packages/agent/test/remote-compaction.test.ts packages/coding-agent/test/compaction-prefer-current-model.test.ts && bun test packages/coding-agent/test/model-registry.test.ts --test-name-pattern "custom Responses providers" (15 pass), package checks for catalog, agent, and coding-agent, and root bun check (passed). Fixes #3104

- Added provider/model remoteCompaction metadata and models.yml propagation.\n- Routed configured OpenAI-compatible compaction endpoints for custom providers.\n- Added compactionModel as a summary-only model selector that leaves the active session model unchanged.\n\nFixes #3104

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b7aefe0689

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

Comment on lines +7618 to +7621
const remoteReady =
Boolean(effectiveSettings.remoteEndpoint) || shouldUseOpenAiRemoteCompaction(this.model);
Boolean(effectiveSettings.remoteEndpoint) ||
shouldUseOpenAiRemoteCompaction(this.model) ||
(compactionTarget ? shouldUseOpenAiRemoteCompaction(compactionTarget) : false);

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Skip local compactionModel for remote mode

When /compact remote is used with no compaction.remoteEndpoint, a non-remote compactionModel plus an active OpenAI/Responses model makes this check report remoteReady via this.model, but #getCompactionModelCandidates() tries the configured compaction model first. That path then runs a local summary in compact() instead of provider-native remote compaction, so the explicit remote mode silently does the opposite; base readiness and candidate selection on the same model, or skip non-remote compaction targets for this mode.

Useful? React with 👍 / 👎.

streamSimple?: (model: Model<Api>, context: Context, options?: SimpleStreamOptions) => AssistantMessageEventStream;
headers?: Record<string, string>;
compat?: ModelSpec<Api>["compat"];
remoteCompaction?: RemoteCompactionConfig<Api>;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply runtime remoteCompaction overrides

Adding remoteCompaction to ProviderConfigInput makes it look usable for extension/runtime provider overrides, but the override-only branch in registerProvider() only runs for baseUrl/headers/apiKey/authHeader/transport and builds a transportOverride without this field. Calls such as registerProvider("openai", { remoteCompaction: { enabled: false } }) or a baseUrl override with compaction metadata therefore leave all existing models unchanged, so runtime providers cannot actually override native compaction unless they also replace every model.

Useful? React with 👍 / 👎.

…rProvider

The runtime override-only branch in ModelRegistry.registerProvider gated on
baseUrl/headers/apiKey/authHeader/transport and built a transportOverride that
omitted remoteCompaction, so calls like
registerProvider('openai', { remoteCompaction: { enabled: false } }) silently
left every existing model's compaction config unchanged.

Added remoteCompaction to the gate and to the override payload — the existing
mergeProviderOverride / applyProviderTransportOverride helpers already merge it
onto runtime override state and models, so wiring is enough. Added a regression
test that drives the override-only path and asserts the propagation survives
refresh and refreshProvider cycles, then is undone by clearSourceRegistrations.

Fixes #3104
@roboomp

roboomp commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed d370b37 addressing the review note.

  • Added config.remoteCompaction !== undefined to the override-only gate in ModelRegistry.registerProvider and propagated remoteCompaction through transportOverride; the existing #mergeProviderOverride / #applyProviderTransportOverride helpers already merge it onto runtime override state and the in-memory model list, so a call like registerProvider("openai", { remoteCompaction: { enabled: false } }) now mutates every existing model.
  • Added a regression test (registerProvider applies remoteCompaction-only overrides to existing provider models across refresh) that drives the override-only path on the bundled anthropic provider, asserts the override survives refresh("offline") and refreshProvider(..., "offline"), and is undone by clearSourceRegistrations. Verified it fails on the prior tree and passes after the fix.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d370b37aa5

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

// ============================================================================

function isOpenAiRemoteCompactionApi(api: Api | undefined): boolean {
return api === "openai-responses" || api === "azure-openai-responses" || api === "openai-codex-responses";

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle Azure compaction with Azure request shape

When a custom/provider model opts in with remoteCompaction.api: "azure-openai-responses", this check sends it through requestOpenAiRemoteCompaction, but that helper still builds an OpenAI-style URL/header set (/v1/responses/compact and Authorization: Bearer). I checked the repo's Azure Responses request builder in packages/ai/src/providers/azure-openai-responses.ts, which uses an api-key header and ?api-version=... on ${baseUrl}/responses; Azure compaction requests using the normal Azure config will therefore fail and fall back to local summarization instead of using the configured native path.

Useful? React with 👍 / 👎.

…ection

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
@roboomp

roboomp commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed aa463f9 addressing the review note.

  • Aligned /compact remote readiness with candidate selection in AgentSession.compact(): when compactMode.requiresRemote is set and no compaction.remoteEndpoint is configured, the candidate chain is filtered through shouldUseOpenAiRemoteCompaction so a non-remote compactionModel is no longer tried first (and no longer triggers a silent local summary on the explicit remote mode); the filter is bypassed when an endpoint is present since compact() will route through it regardless.
  • Threaded the filter through #getCompactionModelCandidates / #resolveCompactionModelCandidates and the resolved candidate list through #compactWithFallbackModel so the readiness check and the actual invocation share one source of truth; the warning now fires only when the filtered chain is empty and we fall back to the unfiltered chain so the operation still completes.
  • Added a regression test in compaction-prefer-current-model.test.ts (/compact remote skips a non-remote-capable compactionModel ...) that wires an OpenAI active model with an Anthropic compactionModel, calls session.compact({ mode: "remote" }), and asserts the OpenAI model — not the configured compactionModel — is the first candidate handed to compact(). Verified failing on the prior tree, passing after the fix; the 13-file compaction test sweep stays green.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: aa463f9008

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

headers: config.headers,
apiKey: config.apiKey,
authHeader: config.authHeader,
remoteCompaction: config.remoteCompaction,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Preserve per-model remote compaction for dynamic providers

When an extension registers fetchDynamicModels with a provider-level remoteCompaction default and a returned model also has its own remoteCompaction (for example a different endpoint or compact model), the fetcher path already merges provider + model config in buildCustomModelOverlay. Storing the same provider config here as a runtime provider override means refreshRuntimeProviders() later calls #applyRuntimeProviderOverrides and merges the provider values over the discovered model values, silently replacing the per-model compact endpoint/model with the provider default. Static config.models returns before this branch, so this regression is specific to dynamic runtime providers.

Useful? React with 👍 / 👎.

Azure Responses remote compaction was enabled by shouldUseOpenAiRemoteCompaction
but requestOpenAiRemoteCompaction still built OpenAI-style requests:
Authorization: Bearer plus a URL without api-version. Azure Responses uses an
api-key header and api-version query parameter, so normal Azure configs failed
and fell back to local summarization.

Derive Azure compact URLs from the Azure Responses base URL/resource-name
configuration, append api-version, and send api-key headers while preserving
custom headers. Existing OpenAI and Codex request shapes are unchanged.

Added a regression test that opts into azure-openai-responses compaction and
asserts the compact URL, api-key auth, absence of Authorization, custom header
preservation, and configured compaction model payload.

Fixes #3104
@roboomp

roboomp commented Jun 20, 2026

Copy link
Copy Markdown
Collaborator Author

Pushed d1066ae addressing the Azure remote-compaction review note.

  • Added Azure-specific compact endpoint resolution in packages/agent/src/compaction/openai.ts: azure-openai-responses now derives ${baseUrl}/responses/compact?api-version=... from AZURE_OPENAI_BASE_URL, AZURE_OPENAI_RESOURCE_NAME, or model.baseUrl, and preserves explicit remoteCompaction.endpoint while adding api-version when missing.
  • Switched Azure remote-compaction requests to Azure auth shape: api-key plus content-type, no Authorization: Bearer, while preserving custom model headers; OpenAI and Codex request shapes are unchanged.
  • Added remote-compaction.test.ts coverage for an azure-openai-responses compaction model asserting URL, api-key, missing Authorization, custom headers, and request body model override; verified the new test fails on the prior code and passes after the fix. packages/agent: bun check and bun test test/remote-compaction.test.ts pass.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: d1066ae4ad

ℹ️ About Codex in GitHub

Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".

opts?: { fetch?: FetchImpl; timeoutMs?: number },
): Promise<OpenAiRemoteCompactionResponse> {
const endpoint = resolveOpenAiCompactEndpoint(model);
const requestModel = model.remoteCompaction?.model ?? model.requestModelId ?? model.id;

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Honor Azure deployment mappings for compaction

When remoteCompaction.api is azure-openai-responses, this falls back to model.id unless remoteCompaction.model is duplicated, but the normal Azure Responses path resolves AZURE_OPENAI_DEPLOYMENT_NAME_MAP and sends that deployment name as the request model (packages/ai/src/providers/azure-openai-responses.ts). In Azure setups where catalog ids like gpt-5 are mapped to deployment names, remote compaction sends the unmapped id, the Azure request fails, and compaction silently falls back to local summarization even though chat requests work with the same model.

Useful? React with 👍 / 👎.

apiKey: providerConfig.apiKey,
authHeader: providerConfig.authHeader,
compat: mergeCompat(providerConfig.compat, disableStrictCompat),
remoteCompaction: providerConfig.remoteCompaction,

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Apply remote compaction overrides to cached discovery models

Adding this provider-level override makes discovery-backed providers look configurable, but warm-start cache loading in #loadCachedDiscoverableModels rebuilds cached rows and only reapplies compat/modelOverrides, while the discovery cache itself is written before this override is merged. With a fresh discovery cache (or before refreshRuntimeDiscoveries runs), models for a custom provider configured only with provider-level remoteCompaction have no remote compaction metadata, so /compact remote falls back locally until a refresh path happens to re-merge the override.

Useful? React with 👍 / 👎.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

vouched Passed the vouch gate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: custom providers cannot use native/remote compaction and promotion does not return to original model

1 participant